diff --git a/CHANGELOG.md b/CHANGELOG.md index c0317fb452..9ea45e1f48 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/sdk/include/opentelemetry/sdk/resource/resource.h b/sdk/include/opentelemetry/sdk/resource/resource.h index c69b971b04..09da0b8c97 100644 --- a/sdk/include/opentelemetry/sdk/resource/resource.h +++ b/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. */ diff --git a/sdk/src/resource/resource.cc b/sdk/src/resource/resource.cc index a07f4d3a3c..f71e17a8a7 100644 --- a/sdk/src/resource/resource.cc +++ b/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) diff --git a/sdk/test/resource/resource_test.cc b/sdk/test/resource/resource_test.cc index 1c14301a57..803d5666d9 100644 --- a/sdk/test/resource/resource_test.cc +++ b/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) {