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 <[email protected]>

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

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (075ff22) to head (58b5837).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3966      +/-   ##
==========================================
- Coverage   90.33%   90.32%   -0.01%     
==========================================
  Files         230      230              
  Lines        7299     7299              
==========================================
- Hits         6593     6592       -1     
- Misses        706      707       +1

see 1 file with indirect coverage changes

🚀 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 <[email protected]>

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

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 <[email protected]>

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 <[email protected]>

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: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.

Signed-off-by: DCchoudhury15 <[email protected]>

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.

marcalff · 2026-04-17T07:41:14Z

+{
+public:
+  ComposableParentThresholdSamplerConfiguration() = default;
+  std::unique_ptr<SamplerConfiguration> root;


In the yaml spec:

ExperimentalComposableParentThresholdSampler: type: - object additionalProperties: false properties: root: $ref: "#/$defs/ExperimentalComposableSampler" description: Sampler to use when there is no parent. required: - root

root is a ComposableSampler, meaning it allows only some types but not all samplers.

For example, root can not point to a jaeger remote sampler, which SamplerConfiguration allows.

Change root to ComposableSamplerConfiguration, which means in turn that classes like ComposableAlwaysOnSamplerConfiguration must be a subclass of ComposableSamplerConfiguration, not SamplerConfiguration.

marcalff · 2026-04-17T07:44:37Z

+  bool match_span_kind_producer{false};
+  bool match_span_kind_consumer{false};
+
+  std::unique_ptr<SamplerConfiguration> sampler;


sampler is a ComposableSamplerConfiguration, not just a SamplerConfiguration

marcalff · 2026-04-17T07:45:25Z

+{
+public:
+  ComposableSamplerConfiguration() = default;
+  std::unique_ptr<SamplerConfiguration> inner;


Remove inner

marcalff · 2026-04-17T07:46:17Z

+public:
+  ComposableSamplerConfiguration() = default;
+  std::unique_ptr<SamplerConfiguration> inner;
+  void Accept(SamplerConfigurationVisitor *visitor) const override;


Remove Accept.

ComposableSamplerConfiguration should be an abstract class, children will implement Accept for each composable sampler type supported.

marcalff · 2026-04-17T07:47:24Z

+  ParseComposableRuleBasedSamplerConfiguration(const std::unique_ptr<DocumentNode> &node,
+                                               size_t depth) const;
+
+  std::unique_ptr<SamplerConfiguration> ParseComposableSamplerConfiguration(


This should return a ComposableSamplerConfiguration.

marcalff · 2026-04-17T07:48:50Z

+  {}
+  virtual void VisitComposableRuleBased(const ComposableRuleBasedSamplerConfiguration * /*model*/)
+  {}
+  virtual void VisitComposable(const ComposableSamplerConfiguration * /*model*/) {}


Remove VisitComposable.

marcalff · 2026-04-17T07:52:50Z

+    const std::unique_ptr<DocumentNode> &node) const
+{
+  auto model = std::make_unique<ComposableRuleBasedSamplerRuleAttributeValuesConfiguration>();
+  model->key = node->GetString("key", "");


key is required, use GetRequiredString.

marcalff · 2026-04-17T07:53:42Z

+{
+  auto model = std::make_unique<ComposableRuleBasedSamplerRuleAttributeValuesConfiguration>();
+  model->key = node->GetString("key", "");
+  auto vals  = node->GetChildNode("values");


values is required.

marcalff · 2026-04-17T07:54:50Z

+    const std::unique_ptr<DocumentNode> &node) const
+{
+  auto model = std::make_unique<ComposableRuleBasedSamplerRuleAttributePatternsConfiguration>();
+  model->key = node->GetString("key", "");


key is required in yaml

marcalff · 2026-04-17T07:56:34Z

+      else if (p_str == "remote")
+        rule->match_parent_remote = true;
+      else if (p_str == "local")
+        rule->match_parent_local = true;


else ... raise an exception for illegal values that do not fit the enum.

Also, please add a unit test for illegal values, for coverage.

marcalff · 2026-04-17T07:57:29Z

+      else if (k_str == "producer")
+        rule->match_span_kind_producer = true;
+      else if (k_str == "consumer")
+        rule->match_span_kind_consumer = true;


Likewise:

else ... raise an exception for illegal values that do not fit the enum.
add a unit test for illegal values, for coverage.

marcalff · 2026-04-17T08:03:14Z

+  TestSamplerVisitor visitor;
+  config->tracer_provider->sampler->Accept(&visitor);
+  EXPECT_EQ(visitor.type_matched, SamplerType::kComposableParentThreshold);
+}


Thanks for the unit tests.

Please also add tests to cover the rule based samplers, for coverage.

marcalff

Thanks for all the changes to date, the yaml parsing itself is almost there.

Typing in the model needs adjustments, the whole point of Sampler versus ComposableSampler is to have strong typing in the yaml schema, so the C++ model classes should represent that.

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 <[email protected]>

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 <[email protected]>

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