feat: add YAML parsing support for Composable Samplers by DCchoudhury15 · Pull Request #3966 · open-telemetry/opentelemetry-cpp

DCchoudhury15 · 2026-04-03T10:02:08Z

Changes

This PR implements the necessary infrastructure and test coverage to fully support composable samplers. Specifically, the changes include:

Parser Logic: Implemented the YAML parsing logic to properly extract composable sampler configurations.
SDK Builder: Added SDK builder visitor instantiation to correctly wire the new sampler variants.
Test Safety: Replaced dangerous casts with the safe Visitor pattern in the testing suite.
Test Coverage: Expanded unit test coverage to ensure all composable sampler variants are accurately parsed and instantiated.

For significant contributions please make sure you have completed the following items:

CHANGELOG.md updated for non-trivial changes
Unit tests have been added
Changes in public API reviewed

Implements parser logic, SDK builder visitor instantiation, safe test patterns, and expands coverage for all composable sampler variants to resolve open-telemetry#3914. Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

codecov · 2026-04-03T10:37:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.18%. Comparing base (0066693) to head (4d117b0).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3966   +/-   ##
=======================================
  Coverage   90.18%   90.18%           
=======================================
  Files         230      230           
  Lines        7299     7299           
=======================================
  Hits         6582     6582           
  Misses        717      717

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

marcalff · 2026-04-03T16:23:22Z

@DCchoudhury15

Thanks for the PR.

Please check the CI logs for failures, and adjust the code accordingly.

In the mean time, I will start the code review and provide feedback.

Thanks for your contribution.

Examples of failures below

Include what you use:

[ 57%] Building CXX object sdk/src/configuration/CMakeFiles/opentelemetry_configuration.dir/yaml_configuration_parser.cc.o
Warning: include-what-you-use reported diagnostics:

/home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/src/configuration/configuration_parser.cc should add these lines:
#include "opentelemetry/sdk/configuration/composable_always_off_sampler_configuration.h"
#include "opentelemetry/sdk/configuration/composable_always_on_sampler_configuration.h"
#include "opentelemetry/sdk/configuration/composable_parent_threshold_sampler_configuration.h"
#include "opentelemetry/sdk/configuration/composable_probability_sampler_configuration.h"
#include "opentelemetry/sdk/configuration/composable_rule_based_sampler_configuration.h"
#include "opentelemetry/sdk/configuration/composable_sampler_configuration.h"

Build in maintainer mode

[ 53%] Building CXX object sdk/src/configuration/CMakeFiles/opentelemetry_configuration.dir/configuration_parser.cc.o
In file included from /home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/src/configuration/configuration_parser.cc:29:
In file included from /home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/include/opentelemetry/sdk/configuration/configuration_parser.h:20:
/home/runner/work/opentelemetry-cpp/opentelemetry-cpp/sdk/include/opentelemetry/sdk/configuration/composable_always_off_sampler_configuration.h:26:28: error: no newline at end of file [-Werror,-Wnewline-eof]
   26 | OPENTELEMETRY_END_NAMESPACE
      |                            ^

Clang-tidy

Download the clang-tidy report from ci, and inspect failures related to new code.

 opentelemetry-cpp/sdk/include/opentelemetry/sdk/configuration/composable_always_off_sampler_configuration.h (1 warnings)Line Check Message15 cppcoreguidelines-special-member-functions class ‘ComposableAlwaysOffSamplerConfiguration’ defines a default destructor but
does not define a copy constructor, a copy assignment operator, a move constructor
or a move assignment operator

DCchoudhury15 · 2026-04-03T21:39:39Z

Hi @marcalff, addressed all the CI failures:

Added direct includes in configuration_parser.cc (IWYU)
Fixed missing newlines at end of the new headers (clang-tidy)
Dropped the explicit destructors since the base class already handles it
Also did some changes in the test visitor for a SamplerType enum while I was at it.
Thanks for the patience!

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

lalitb · 2026-04-08T00:40:26Z

  {
    model = ParseTraceIdRatioBasedSamplerConfiguration(child, depth);
  }
+  else if (name == "composable_always_off")


This is confusing to me - As per the config specs, the YAML shape should be something like:

tracer_provider: sampler: composite/development: always_on:

However in the implementation we are using composable_* sampler keys, which seems incorrect. Or am I missing something.

Lalit is correct here, the only name to check should be "composite/development".

In this case, invoke ParseComposableSamplerConfiguration(), which will lookup the child yaml nodes.

marcalff · 2026-04-08T15:47:46Z

+    size_t /* depth */) const
+{
+  auto model         = std::make_unique<ComposableProbabilitySamplerConfiguration>();
+  model->probability = node->GetDouble("probability", 1.0);


The property name in the spec is ratio.

marcalff · 2026-04-08T15:48:52Z

+    const std::unique_ptr<DocumentNode> & /* node */,
+    size_t /* depth */) const
+{
+  return std::make_unique<ComposableParentThresholdSamplerConfiguration>();


Property root is not parsed from yaml.

marcalff · 2026-04-08T15:50:03Z

+    const std::unique_ptr<DocumentNode> & /* node */,
+    size_t /* depth */) const
+{
+  // FIXME-CONFIG: Parse rules list from YAML node here


To implement, parse the array of rules from yaml.

marcalff · 2026-04-08T15:53:28Z

+    const std::unique_ptr<DocumentNode> & /* node */,
+    size_t /* depth */) const
+{
+  return std::make_unique<ComposableSamplerConfiguration>();


This is an empty node.

The code needs to parse the child nodes in yaml, and create the proper sampler.

See

minProperties: 1 maxProperties: 1 properties: always_off: $ref: "#/$defs/ExperimentalComposableAlwaysOffSampler" description: Configure sampler to be always_off. defaultBehavior: ignore always_on: $ref: "#/$defs/ExperimentalComposableAlwaysOnSampler" description: Configure sampler to be always_on. defaultBehavior: ignore parent_threshold: $ref: "#/$defs/ExperimentalComposableParentThresholdSampler" description: | Configure sampler to be parent_threshold. defaultBehavior: ignore probability: $ref: "#/$defs/ExperimentalComposableProbabilitySampler" description: Configure sampler to be probability. defaultBehavior: ignore rule_based: $ref: "#/$defs/ExperimentalComposableRuleBasedSampler" description: Configure sampler to be rule_based. defaultBehavior: ignore

marcalff

Thanks for the PR.

Focusing on the yaml part for now: all the yaml nodes and properties should be parsed, and represented in memory in C++ classes.

This includes rules with attribute_values, attribute_patterns and span_kinds.

marcalff · 2026-04-08T15:58:29Z

  {
    model = ParseTraceIdRatioBasedSamplerConfiguration(child, depth);
  }
+  else if (name == "composable_always_off")


Lalit is correct here, the only name to check should be "composite/development".

In this case, invoke ParseComposableSamplerConfiguration(), which will lookup the child yaml nodes.

marcalff · 2026-04-08T16:19:30Z

+  sampler:
+    composable_always_on:


This is not the expected schema.

See file snippets/Sampler_rule_based_kitchen_sink.yaml in the opentelemetry-configuration repository.

tracer_provider: processors: - simple: exporter: console: sampler: # SNIPPET_START composite/development: ...

DCchoudhury15 · 2026-04-10T12:03:23Z

I have applied the changes given on the pr review , will resolve all the ci errors and commit again asap .

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

ThomsonTan · 2026-04-13T18:20:54Z

+  virtual void VisitParentBased(const ParentBasedSamplerConfiguration *model)                 = 0;
+  virtual void VisitTraceIdRatioBased(const TraceIdRatioBasedSamplerConfiguration *model)     = 0;
+  virtual void VisitExtension(const ExtensionSamplerConfiguration *model)                     = 0;
+  virtual void VisitComposableAlwaysOff(const ComposableAlwaysOffSamplerConfiguration *model) = 0;


Provide default implementation to avoid breaking downstream users for these 2 virtual functions?

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

marcalff

This is getting closer, thanks for the changes.

See some comments on the structure.

marcalff · 2026-04-15T10:47:51Z

+  auto model = std::make_unique<ComposableParentThresholdSamplerConfiguration>();
+  std::unique_ptr<DocumentNode> child;
+
+  child = node->GetChildNode("root");


Property root is required, not optional

marcalff · 2026-04-15T10:57:11Z

+  if (rules_node)
+  {
+    for (auto it = rules_node->begin(); it != rules_node->end(); ++it)
+    {


Please extract the body of the for loop into a separate method, ParseComposableRuleBasedSamplerRuleConfiguration()

marcalff · 2026-04-15T10:59:55Z

+      }
+
+      // parse the rule's sampler
+      std::unique_ptr<DocumentNode> sampler = rule_node->GetChildNode("sampler");


property sampler is required, not optional.

marcalff · 2026-04-15T13:23:42Z

+    auto inner_child       = it.Value();
+
+    if (inner_name == "always_off")
+      model->inner = ParseComposableAlwaysOffSamplerConfiguration(inner_child, depth);


This returns an instance of ComposableSamplerConfiguration, which points to a different instance of ComposableAlwaysOffConfiguration, using the inner pointer.

Instead, return directly the ComposableAlwaysOffConfiguration object, as a sub class of ComposableSamplerConfiguration.

marcalff · 2026-04-15T15:40:01Z

+  std::unique_ptr<ComposableRuleBasedSamplerRuleAttributeValuesConfiguration> attribute_values;
+  std::unique_ptr<ComposableRuleBasedSamplerRuleAttributePatternsConfiguration> attribute_patterns;
+  std::vector<std::string> parent;
+  std::vector<std::string> span_kinds;


span_kinds is an enum in the yaml schema:

SpanKind: type: - string - "null" enum: - internal - server - client - producer - consumer

Define a C++ enum for this, and use a vector of enums.

The code will need helpers to convert a string to the enum as well.

[edit]

Or better, define 5 booleans (match_span_kind_internal, match_span_kind_server, etc)

marcalff · 2026-04-15T15:45:42Z

+      std::unique_ptr<DocumentNode> av = rule_node->GetChildNode("attribute_values");
+      if (av)
+      {
+        auto avc  = std::make_unique<ComposableRuleBasedSamplerRuleAttributeValuesConfiguration>();


Likewise, extract this code into a helper.

The philosophy is: each yaml type should have a corresponding Parse method.

This helps in the long term, because new yaml nodes can reuse existing types, and then the parsing code for the new node just reuses the existing Parse method for the type.

marcalff · 2026-04-15T15:49:30Z

+      model->inner = ParseComposableParentThresholdSamplerConfiguration(inner_child, depth);
+    else if (inner_name == "rule_based")
+      model->inner = ParseComposableRuleBasedSamplerConfiguration(inner_child, depth);
+  }


This code should also enforce that there is only one child, and throw yaml parsing exceptions if not.

marcalff · 2026-04-15T15:55:18Z

+
+  std::unique_ptr<ComposableRuleBasedSamplerRuleAttributeValuesConfiguration> attribute_values;
+  std::unique_ptr<ComposableRuleBasedSamplerRuleAttributePatternsConfiguration> attribute_patterns;
+  std::vector<std::string> parent;


parent should be an enum, not a string.

But, on top of that, keeping an array of enums with duplicates is not practical.

Please replace with 3 booleans:

match_parent_none

match_parent_remote

match_parent_local

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

DCchoudhury15 · 2026-04-16T05:33:27Z

Hi @marcalff , thank you for the detailed review and your patience , I cleaned up the YAML parsing logic by extracting helper methods and strictly enforcing required nodes to ensure it fails fast on bad schemas. I also swapped out the string arrays for boolean flags to make the enum matching much more performant. Finally, I fixed the -Werror unused parameter warnings that were breaking all the maintainer-mode CI builds by safely commenting out those variables in the visitor header.

DCchoudhury15 requested a review from a team as a code owner April 3, 2026 10:02

feat: add YAML parsing support for Composable Samplers

ae0c9dd

Implements parser logic, SDK builder visitor instantiation, safe test patterns, and expands coverage for all composable sampler variants to resolve open-telemetry#3914. Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

DCchoudhury15 force-pushed the feat/config-composable-samplers branch from 92ed254 to ae0c9dd Compare April 3, 2026 10:09

fix CI errors and formatting issues

659aafe

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

DCchoudhury15 force-pushed the feat/config-composable-samplers branch from f5be4bf to 659aafe Compare April 4, 2026 07:24

lalitb and others added 3 commits April 4, 2026 08:05

Merge branch 'main' into feat/config-composable-samplers

4f801aa

Merge branch 'main' into feat/config-composable-samplers

813a6d4

Merge branch 'main' into feat/config-composable-samplers

4cf8d39

ThomsonTan reviewed Apr 7, 2026

View reviewed changes

Comment thread sdk/test/configuration/yaml_trace_test.cc

lalitb reviewed Apr 8, 2026

View reviewed changes

marcalff reviewed Apr 8, 2026

View reviewed changes

DCchoudhury15 force-pushed the feat/config-composable-samplers branch from 2e15234 to 758e3a5 Compare April 10, 2026 05:48

fix: address review feedback for composable samplers YAML parsing

1a4617d

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

DCchoudhury15 force-pushed the feat/config-composable-samplers branch from 13f24d4 to 1a4617d Compare April 11, 2026 01:07

Merge branch 'main' into feat/config-composable-samplers

e22556d

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

DCchoudhury15 force-pushed the feat/config-composable-samplers branch from e207db5 to e22556d Compare April 13, 2026 04:32

Merge branch 'main' into feat/config-composable-samplers

30be1a0

ThomsonTan reviewed Apr 13, 2026

View reviewed changes

DCchoudhury15 force-pushed the feat/config-composable-samplers branch 2 times, most recently from 7b65db4 to b96bd0b Compare April 15, 2026 04:46

Fix clang-tidy recursion warnings and gtest include order

db8b88f

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

DCchoudhury15 force-pushed the feat/config-composable-samplers branch from 05e42ce to db8b88f Compare April 15, 2026 05:45

Merge branch 'main' into feat/config-composable-samplers

5b3d911

marcalff requested changes Apr 15, 2026

View reviewed changes

marcalff reviewed Apr 15, 2026

View reviewed changes

DCchoudhury15 added 2 commits April 16, 2026 10:53

Merge branch 'main' into feat/config-composable-samplers

e8750da

fix: address maintainer review feedback and unused parameter warnings

4d117b0

Signed-off-by: DCchoudhury15 <divyanshuchoudhury3@gmail.com>

DCchoudhury15 force-pushed the feat/config-composable-samplers branch from 8363bcc to 4d117b0 Compare April 16, 2026 05:28

DCchoudhury15 added 2 commits April 16, 2026 12:50

Merge branch 'main' into feat/config-composable-samplers

f91eb58

Merge branch 'main' into feat/config-composable-samplers

678b6f9

Conversation

DCchoudhury15 commented Apr 3, 2026

Changes

Uh oh!

codecov bot commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

marcalff commented Apr 3, 2026

Uh oh!

DCchoudhury15 commented Apr 3, 2026

Uh oh!

Uh oh!

lalitb Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcalff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DCchoudhury15 commented Apr 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcalff left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcalff Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcalff Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DCchoudhury15 commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Apr 3, 2026 •

edited

Loading

lalitb Apr 8, 2026 •

edited

Loading

marcalff Apr 15, 2026 •

edited

Loading

marcalff Apr 15, 2026 •

edited

Loading