From 6d2de7c87aeddd710a7c3c7827056f0278c71d17 Mon Sep 17 00:00:00 2001 From: Johan Peltenburg Date: Thu, 9 Mar 2023 13:31:22 +0100 Subject: [PATCH 1/3] Pick schema_url according to specification in Resource::Merge --- .../opentelemetry/sdk/resource/resource.h | 4 +++ sdk/src/resource/resource.cc | 3 +- sdk/test/resource/resource_test.cc | 30 ++++++++++++++++++- 3 files changed, 35 insertions(+), 2 deletions(-) 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) { From e39d3cc6ab4d5545e3d1243f017ed49fbd31c10e Mon Sep 17 00:00:00 2001 From: Johan Peltenburg Date: Thu, 9 Mar 2023 14:30:21 +0100 Subject: [PATCH 2/3] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d95b4a5f0..56f7c5b1d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ Increment the: * PATCH version when you make backwards compatible bug fixes. ## [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 From 005dc6d1814b0589842684667c2c2db6c90125fe Mon Sep 17 00:00:00 2001 From: Johan Peltenburg Date: Thu, 9 Mar 2023 14:44:56 +0100 Subject: [PATCH 3/3] Add blank line above markdown list --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 56f7c5b1d4..4177a3abb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Increment the: * PATCH version when you make backwards compatible bug fixes. ## [Unreleased] + * [RESOURCE SDK] Fix schema URL precedence bug in `Resource::Merge`. [#2036](https://github.com/open-telemetry/opentelemetry-cpp/pull/2036)