Merge branch 'main' into FixOOB

CHANGELOG.md

@@ -15,6 +15,9 @@ Increment the:
 
 ## [Unreleased]
 
+* [RESOURCE SDK] Fix schema URL precedence bug in `Resource::Merge`.
+  [#2036](https://github.com/open-telemetry/opentelemetry-cpp/pull/2036)
+
 ## [1.8.3] 2023-03-06
 
 * Provide version major/minor/patch macros

sdk/include/opentelemetry/sdk/resource/resource.h

@@ -36,6 +36,10 @@ class Resource
    * with the other Resource. In case of a collision, the other Resource takes
    * precedence.
    *
+   * The specification notes that if schema urls collide, the resulting
+   * schema url is implementation-defined. In the C++ implementation, the
+   * schema url of @param other is picked.
+   *
    * @param other the Resource that will be merged with this.
    * @returns the newly merged Resource.
    */

sdk/src/resource/resource.cc

@@ -21,7 +21,8 @@ Resource Resource::Merge(const Resource &other) const noexcept
 {
   ResourceAttributes merged_resource_attributes(other.attributes_);
   merged_resource_attributes.insert(attributes_.begin(), attributes_.end());
-  return Resource(merged_resource_attributes, other.schema_url_);
+  return Resource(merged_resource_attributes,
+                  other.schema_url_.empty() ? schema_url_ : other.schema_url_);
 }
 
 Resource Resource::Create(const ResourceAttributes &attributes, const std::string &schema_url)

sdk/test/resource/resource_test.cc

@@ -26,7 +26,9 @@ namespace nostd = opentelemetry::nostd;
 class TestResource : public Resource
 {
 public:
-  TestResource(ResourceAttributes attributes = ResourceAttributes()) : Resource(attributes) {}
+  TestResource(ResourceAttributes attributes = ResourceAttributes(), std::string schema_url = {})
+      : Resource(attributes, schema_url)
+  {}
 };
 
 TEST(ResourceTest, create_without_servicename)
@@ -170,6 +172,32 @@ TEST(ResourceTest, MergeEmptyString)
   EXPECT_EQ(received_attributes.size(), expected_attributes.size());
 }
 
+TEST(ResourceTest, MergeSchemaUrl)
+{
+  const std::string url = "https://opentelemetry.io/schemas/v3.1.4";
+
+  TestResource resource_empty_url({}, "");
+  TestResource resource_some_url({}, url);
+  TestResource resource_different_url({}, "different");
+
+  // Specified behavior:
+  auto merged_both_empty = resource_empty_url.Merge(resource_empty_url);
+  EXPECT_TRUE(merged_both_empty.GetSchemaURL().empty());
+
+  auto merged_old_empty = resource_empty_url.Merge(resource_some_url);
+  EXPECT_EQ(merged_old_empty.GetSchemaURL(), url);
+
+  auto merged_updating_empty = resource_some_url.Merge(resource_empty_url);
+  EXPECT_EQ(merged_updating_empty.GetSchemaURL(), url);
+
+  auto merged_same_url = resource_some_url.Merge(resource_some_url);
+  EXPECT_EQ(merged_same_url.GetSchemaURL(), url);
+
+  // Implementation-defined behavior:
+  auto merged_different_url = resource_different_url.Merge(resource_some_url);
+  EXPECT_EQ(merged_different_url.GetSchemaURL(), url);
+}
+
 #ifndef NO_GETENV
 TEST(ResourceTest, OtelResourceDetector)
 {