Rework for propagators, added tests.

Author: marcalff

functional/configuration/shelltests/kitchen-sink.yaml

@@ -562,7 +562,11 @@ propagator:
   # The value is a comma separated list of propagator identifiers matching the format of OTEL_PROPAGATORS. See https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/configuration/sdk-environment-variables.md#general-sdk-configuration for details.
   # Built-in propagator identifiers include: tracecontext, baggage, b3, b3multi, jaeger, ottrace. Known third party identifiers include: xray.
   # If the resolved list of propagators (from .composite and .composite_list) is empty, a noop propagator is used.
-  composite_list: "tracecontext,baggage,b3,b3multi,jaeger,ottrace,xray"
+#  composite_list: "tracecontext,baggage,b3,b3multi,jaeger,ottrace,xray"
+
+# ottrace, xray not supported in opentelemetry-cpp
+  composite_list: "tracecontext,baggage,b3,b3multi,jaeger"
+
 # Configure tracer provider.
 # If omitted, a noop tracer provider is used.
 tracer_provider:

functional/configuration/shelltests/propagator_both.test

@@ -0,0 +1,8 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+$ otel_configuration_check --yaml shelltests/propagator_both.yaml
+>
+MODEL PARSED
+SDK CREATED
+>= 0

functional/configuration/shelltests/propagator_both.yaml

@@ -0,0 +1,10 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+file_format: 0.0
+
+propagator:
+  composite:
+    - tracecontext:
+    - baggage:
+  composite_list: "b3,b3multi"

functional/configuration/shelltests/propagator_composite_list.test

@@ -0,0 +1,8 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+$ otel_configuration_check --yaml shelltests/propagator_composite_list.yaml
+>
+MODEL PARSED
+SDK CREATED
+>= 0

functional/configuration/shelltests/propagator_composite_list.yaml

@@ -0,0 +1,7 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+file_format: 0.0
+
+propagator:
+  composite_list: "tracecontext,baggage,b3,b3multi"

functional/configuration/shelltests/propagator_duplicates.test

@@ -0,0 +1,8 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+$ otel_configuration_check --yaml shelltests/propagator_duplicates.yaml
+>
+MODEL PARSED
+SDK CREATED
+>= 0

functional/configuration/shelltests/propagator_duplicates.yaml

@@ -0,0 +1,10 @@
+# Copyright The OpenTelemetry Authors
+# SPDX-License-Identifier: Apache-2.0
+
+file_format: 0.0
+
+propagator:
+  composite:
+    - tracecontext:
+    - baggage:
+  composite_list: "tracecontext,baggage,baggage,b3,b3multi"

sdk/src/configuration/sdk_builder.cc

@@ -1061,42 +1061,87 @@ SdkBuilder::CreateTextMapPropagator(const std::string &name) const
   throw UnsupportedException(die);
 }
 
+static bool IsDuplicate(const std::vector<std::string> &propagator_seen, const std::string &name)
+{
+  bool duplicate = false;
+  for (const auto &seen : propagator_seen)
+  {
+    if (name == seen)
+    {
+      duplicate = true;
+    }
+  }
+
+  return duplicate;
+}
+
 std::unique_ptr<opentelemetry::context::propagation::TextMapPropagator>
 SdkBuilder::CreatePropagator(
     const std::unique_ptr<opentelemetry::sdk::configuration::PropagatorConfiguration> &model) const
 {
+  std::unique_ptr<opentelemetry::context::propagation::CompositePropagator> sdk;
   std::vector<std::unique_ptr<opentelemetry::context::propagation::TextMapPropagator>> propagators;
   std::unique_ptr<opentelemetry::context::propagation::TextMapPropagator> propagator;
+  std::vector<std::string> propagator_seen;
+  bool duplicate;
+
+  /*
+   * Note that the spec only requires to check duplicates between
+   * composite and composite_list.
+   * Here we check for duplicates globally, for ease of use.
+   */
 
   for (const auto &name : model->composite)
   {
-    propagator = CreateTextMapPropagator(name);
-    propagators.push_back(std::move(propagator));
+    duplicate = IsDuplicate(propagator_seen, name);
+
+    if (!duplicate)
+    {
+      propagator = CreateTextMapPropagator(name);
+      propagators.push_back(std::move(propagator));
+      propagator_seen.push_back(name);
+    }
   }
 
   if (model->composite_list.size() > 0)
   {
     std::string str_list = model->composite_list;
-    size_t start_pos = 0;
-    size_t end_pos = 0;
-    char separator = ',';
+    size_t start_pos     = 0;
+    size_t end_pos       = 0;
+    char separator       = ',';
     std::string name;
 
     while ((end_pos = str_list.find(separator, start_pos)) != std::string::npos)
     {
       name = str_list.substr(start_pos, end_pos - start_pos);
-      propagator = CreateTextMapPropagator(name);
-      propagators.push_back(std::move(propagator));
+
+      duplicate = IsDuplicate(propagator_seen, name);
+
+      if (!duplicate)
+      {
+        propagator = CreateTextMapPropagator(name);
+        propagators.push_back(std::move(propagator));
+        propagator_seen.push_back(name);
+      }
       start_pos = end_pos + 1;
     }
 
     name = str_list.substr(start_pos);
-    propagator = CreateTextMapPropagator(name);
-    propagators.push_back(std::move(propagator));
+
+    duplicate = IsDuplicate(propagator_seen, name);
+
+    if (!duplicate)
+    {
+      propagator = CreateTextMapPropagator(name);
+      propagators.push_back(std::move(propagator));
+    }
   }
 
-  auto sdk = std::make_unique<opentelemetry::context::propagation::CompositePropagator>(
-      std::move(propagators));
+  if (propagators.size() > 0)
+  {
+    sdk = std::make_unique<opentelemetry::context::propagation::CompositePropagator>(
+        std::move(propagators));
+  }
 
   return sdk;
 }

sdk/test/configuration/yaml_propagator_test.cc

@@ -146,3 +146,27 @@ file_format: xx.yy
   ASSERT_EQ(config->propagator->composite[2], "ccc");
   ASSERT_EQ(config->propagator->composite_list, "ddd,eee,fff");
 }
+
+TEST(YamlPropagator, propagator_duplicates)
+{
+  std::string yaml = R"(
+file_format: xx.yy
+propagator:
+  composite:
+    - aaa:
+    - bbb:
+    - bbb:
+    - ccc:
+  composite_list: "aaa,eee,eee,fff,ccc"
+)";
+
+  auto config = DoParse(yaml);
+  ASSERT_NE(config, nullptr);
+  ASSERT_NE(config->propagator, nullptr);
+  ASSERT_EQ(config->propagator->composite.size(), 4);
+  ASSERT_EQ(config->propagator->composite[0], "aaa");
+  ASSERT_EQ(config->propagator->composite[1], "bbb");
+  ASSERT_EQ(config->propagator->composite[2], "bbb");
+  ASSERT_EQ(config->propagator->composite[3], "ccc");
+  ASSERT_EQ(config->propagator->composite_list, "aaa,eee,eee,fff,ccc");
+}