From b93518f349a2b9a048f9d308c71ee54b6033ac85 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Fri, 21 May 2021 12:54:20 -0700 Subject: [PATCH 01/32] Add Resources.schema_url to Resource.create test --- .../tests/resources/test_resources.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 211187f1ebb..f8c3f064940 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -51,6 +51,13 @@ def test_create(self): self.assertIsInstance(resource, resources.Resource) self.assertEqual(resource.attributes, expected_attributes) + schema_url = "https://opentelemetry.io/schemas/1.3.0" + + resource = resources.Resource.create(attributes, schema_url) + self.assertIsInstance(resource, resources.Resource) + self.assertEqual(resource.attributes, expected_attributes) + self.assertEqual(resource.schema_url, schema_url) + os.environ[resources.OTEL_RESOURCE_ATTRIBUTES] = "key=value" resource = resources.Resource.create(attributes) self.assertIsInstance(resource, resources.Resource) @@ -69,6 +76,16 @@ def test_create(self): resources.Resource({resources.SERVICE_NAME: "unknown_service"}) ), ) + self.assertEqual(resource.schema_url, "") + + resource = resources.Resource.create(None, None) + self.assertEqual( + resource, + resources._DEFAULT_RESOURCE.merge( + resources.Resource({resources.SERVICE_NAME: "unknown_service"}) + ), + ) + self.assertEqual(resource.schema_url, "") resource = resources.Resource.create({}) self.assertEqual( @@ -77,6 +94,16 @@ def test_create(self): resources.Resource({resources.SERVICE_NAME: "unknown_service"}) ), ) + self.assertEqual(resource.schema_url, "") + + resource = resources.Resource.create({}, None) + self.assertEqual( + resource, + resources._DEFAULT_RESOURCE.merge( + resources.Resource({resources.SERVICE_NAME: "unknown_service"}) + ), + ) + self.assertEqual(resource.schema_url, "") def test_resource_merge(self): left = resources.Resource({"service": "ui"}) From f5246af6bef253d36ed385d7c438f0964a77e107 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Fri, 21 May 2021 15:37:45 -0700 Subject: [PATCH 02/32] Add Resources.schema_url to Resource.merge test --- .../tests/resources/test_resources.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index f8c3f064940..3e3b9facf43 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -112,6 +112,31 @@ def test_resource_merge(self): left.merge(right), resources.Resource({"service": "ui", "host": "service-host"}), ) + schema_urls = ( + "https://opentelemetry.io/schemas/1.2.0", + "https://opentelemetry.io/schemas/1.3.0", + ) + + left = resources.Resource.create({}, None) + right = resources.Resource.create({}, None) + self.assertEqual(left.merge(right).schema_url, "") + + left = resources.Resource.create({}, None) + right = resources.Resource.create({}, schema_urls[0]) + self.assertEqual(left.merge(right).schema_url, schema_urls[0]) + + left = resources.Resource.create({}, schema_urls[0]) + right = resources.Resource.create({}, None) + self.assertEqual(left.merge(right).schema_url, schema_urls[0]) + + left = resources.Resource.create({}, schema_urls[0]) + right = resources.Resource.create({}, schema_urls[0]) + self.assertEqual(left.merge(right).schema_url, schema_urls[0]) + + left = resources.Resource.create({}, schema_urls[0]) + right = resources.Resource.create({}, schema_urls[1]) + with self.assertRaises(ValueError): + left.merge(right) def test_resource_merge_empty_string(self): """Verify Resource.merge behavior with the empty string. From e1ca938056434d56fd80daa0e4b116f4b7cfe5e9 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Fri, 21 May 2021 16:03:19 -0700 Subject: [PATCH 03/32] Implement schema_url field in Resource --- .../tests/test_otlp_trace_exporter.py | 2 +- .../opentelemetry/sdk/resources/__init__.py | 40 +++++++++--- .../benchmarks/trace/test_benchmark_trace.py | 3 +- .../tests/resources/test_resources.py | 62 ++++++++++++------- opentelemetry-sdk/tests/trace/test_trace.py | 8 +-- 5 files changed, 78 insertions(+), 37 deletions(-) diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py index 3f43f1a8ccc..9574196836d 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py @@ -149,7 +149,7 @@ def setUp(self): "trace_id": 67545097771067222548457157018666467027, } ), - resource=SDKResource(OrderedDict([("a", 1), ("b", False)])), + resource=SDKResource(OrderedDict([("a", 1), ("b", False)]), ""), parent=Mock(**{"span_id": 12345}), attributes=OrderedDict([("a", 1), ("b", True)]), events=[event_mock], diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 77240b8ef8e..2d317f34e25 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -140,11 +140,16 @@ class Resource: """A Resource is an immutable representation of the entity producing telemetry as Attributes.""" - def __init__(self, attributes: Attributes): + def __init__(self, attributes: Attributes, schema_url: str): self._attributes = attributes.copy() + assert schema_url is not None + self._schema_url = schema_url @staticmethod - def create(attributes: typing.Optional[Attributes] = None) -> "Resource": + def create( + attributes: typing.Optional[Attributes] = None, + schema_url: typing.Optional[str] = None, + ) -> "Resource": """Creates a new `Resource` from attributes. Args: @@ -155,9 +160,11 @@ def create(attributes: typing.Optional[Attributes] = None) -> "Resource": """ if not attributes: attributes = {} + if not schema_url: + schema_url = "" resource = _DEFAULT_RESOURCE.merge( OTELResourceDetector().detect() - ).merge(Resource(attributes)) + ).merge(Resource(attributes, schema_url)) if not resource.attributes.get(SERVICE_NAME, None): default_service_name = "unknown_service" process_executable_name = resource.attributes.get( @@ -166,7 +173,7 @@ def create(attributes: typing.Optional[Attributes] = None) -> "Resource": if process_executable_name: default_service_name += ":" + process_executable_name resource = resource.merge( - Resource({SERVICE_NAME: default_service_name}) + Resource({SERVICE_NAME: default_service_name}, schema_url) ) return resource @@ -178,6 +185,10 @@ def get_empty() -> "Resource": def attributes(self) -> Attributes: return self._attributes.copy() + @property + def schema_url(self) -> Attributes: + return self._schema_url + def merge(self, other: "Resource") -> "Resource": """Merges this resource and an updating resource into a new `Resource`. @@ -192,7 +203,19 @@ def merge(self, other: "Resource") -> "Resource": """ merged_attributes = self.attributes merged_attributes.update(other.attributes) - return Resource(merged_attributes) + + if self.schema_url == "": + schema_url = other.schema_url + elif other.schema_url == "": + schema_url = self.schema_url + elif self.schema_url == other.schema_url: + schema_url = other.schema_url + else: + raise ValueError( + "The Schema URL of the old and updating resources are not empty and are different" + ) + + return Resource(merged_attributes, schema_url) def __eq__(self, other: object) -> bool: if not isinstance(other, Resource): @@ -203,13 +226,14 @@ def __hash__(self): return hash(dumps(self._attributes, sort_keys=True)) -_EMPTY_RESOURCE = Resource({}) +_EMPTY_RESOURCE = Resource({}, "") _DEFAULT_RESOURCE = Resource( { TELEMETRY_SDK_LANGUAGE: "python", TELEMETRY_SDK_NAME: "opentelemetry", TELEMETRY_SDK_VERSION: _OPENTELEMETRY_SDK_VERSION, - } + }, + "", ) @@ -237,7 +261,7 @@ def detect(self) -> "Resource": service_name = os.environ.get(OTEL_SERVICE_NAME) if service_name: env_resource_map[SERVICE_NAME] = service_name - return Resource(env_resource_map) + return Resource(env_resource_map, "") def get_aggregated_resources( diff --git a/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py b/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py index a407a341f45..c37a5409da8 100644 --- a/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py +++ b/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py @@ -23,7 +23,8 @@ "service.name": "A123456789", "service.version": "1.34567890", "service.instance.id": "123ab456-a123-12ab-12ab-12340a1abc12", - } + }, + "", ), ).get_tracer("sdk_tracer_provider") diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 3e3b9facf43..032af4d6853 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -73,7 +73,9 @@ def test_create(self): self.assertEqual( resource, resources._DEFAULT_RESOURCE.merge( - resources.Resource({resources.SERVICE_NAME: "unknown_service"}) + resources.Resource( + {resources.SERVICE_NAME: "unknown_service"}, "" + ) ), ) self.assertEqual(resource.schema_url, "") @@ -82,7 +84,9 @@ def test_create(self): self.assertEqual( resource, resources._DEFAULT_RESOURCE.merge( - resources.Resource({resources.SERVICE_NAME: "unknown_service"}) + resources.Resource( + {resources.SERVICE_NAME: "unknown_service"}, "" + ) ), ) self.assertEqual(resource.schema_url, "") @@ -91,7 +95,9 @@ def test_create(self): self.assertEqual( resource, resources._DEFAULT_RESOURCE.merge( - resources.Resource({resources.SERVICE_NAME: "unknown_service"}) + resources.Resource( + {resources.SERVICE_NAME: "unknown_service"}, "" + ) ), ) self.assertEqual(resource.schema_url, "") @@ -100,17 +106,19 @@ def test_create(self): self.assertEqual( resource, resources._DEFAULT_RESOURCE.merge( - resources.Resource({resources.SERVICE_NAME: "unknown_service"}) + resources.Resource( + {resources.SERVICE_NAME: "unknown_service"}, "" + ) ), ) self.assertEqual(resource.schema_url, "") def test_resource_merge(self): - left = resources.Resource({"service": "ui"}) - right = resources.Resource({"host": "service-host"}) + left = resources.Resource({"service": "ui"}, "") + right = resources.Resource({"host": "service-host"}, "") self.assertEqual( left.merge(right), - resources.Resource({"service": "ui", "host": "service-host"}), + resources.Resource({"service": "ui", "host": "service-host"}, ""), ) schema_urls = ( "https://opentelemetry.io/schemas/1.2.0", @@ -145,13 +153,15 @@ def test_resource_merge_empty_string(self): the exception of the empty string. """ - left = resources.Resource({"service": "ui", "host": ""}) + left = resources.Resource({"service": "ui", "host": ""}, "") right = resources.Resource( - {"host": "service-host", "service": "not-ui"} + {"host": "service-host", "service": "not-ui"}, "" ) self.assertEqual( left.merge(right), - resources.Resource({"service": "not-ui", "host": "service-host"}), + resources.Resource( + {"service": "not-ui", "host": "service-host"}, "" + ), ) def test_immutability(self): @@ -195,7 +205,9 @@ def test_aggregated_resources_no_detectors(self): self.assertEqual(aggregated_resources, resources.Resource.get_empty()) def test_aggregated_resources_with_static_resource(self): - static_resource = resources.Resource({"static_key": "static_value"}) + static_resource = resources.Resource( + {"static_key": "static_value"}, "" + ) self.assertEqual( resources.get_aggregated_resources( @@ -206,7 +218,8 @@ def test_aggregated_resources_with_static_resource(self): resource_detector = mock.Mock(spec=resources.ResourceDetector) resource_detector.detect.return_value = resources.Resource( - {"static_key": "try_to_overwrite_existing_value", "key": "value"} + {"static_key": "try_to_overwrite_existing_value", "key": "value"}, + "", ) self.assertEqual( resources.get_aggregated_resources( @@ -216,18 +229,19 @@ def test_aggregated_resources_with_static_resource(self): { "static_key": "try_to_overwrite_existing_value", "key": "value", - } + }, + "", ), ) def test_aggregated_resources_multiple_detectors(self): resource_detector1 = mock.Mock(spec=resources.ResourceDetector) resource_detector1.detect.return_value = resources.Resource( - {"key1": "value1"} + {"key1": "value1"}, "" ) resource_detector2 = mock.Mock(spec=resources.ResourceDetector) resource_detector2.detect.return_value = resources.Resource( - {"key2": "value2", "key3": "value3"} + {"key2": "value2", "key3": "value3"}, "" ) resource_detector3 = mock.Mock(spec=resources.ResourceDetector) resource_detector3.detect.return_value = resources.Resource( @@ -235,7 +249,8 @@ def test_aggregated_resources_multiple_detectors(self): "key2": "try_to_overwrite_existing_value", "key3": "try_to_overwrite_existing_value", "key4": "value4", - } + }, + "", ) self.assertEqual( @@ -248,7 +263,8 @@ def test_aggregated_resources_multiple_detectors(self): "key2": "try_to_overwrite_existing_value", "key3": "try_to_overwrite_existing_value", "key4": "value4", - } + }, + "", ), ) @@ -314,18 +330,18 @@ def test_empty(self): def test_one(self): detector = resources.OTELResourceDetector() os.environ[resources.OTEL_RESOURCE_ATTRIBUTES] = "k=v" - self.assertEqual(detector.detect(), resources.Resource({"k": "v"})) + self.assertEqual(detector.detect(), resources.Resource({"k": "v"}, "")) def test_one_with_whitespace(self): detector = resources.OTELResourceDetector() os.environ[resources.OTEL_RESOURCE_ATTRIBUTES] = " k = v " - self.assertEqual(detector.detect(), resources.Resource({"k": "v"})) + self.assertEqual(detector.detect(), resources.Resource({"k": "v"}, "")) def test_multiple(self): detector = resources.OTELResourceDetector() os.environ[resources.OTEL_RESOURCE_ATTRIBUTES] = "k=v,k2=v2" self.assertEqual( - detector.detect(), resources.Resource({"k": "v", "k2": "v2"}) + detector.detect(), resources.Resource({"k": "v", "k2": "v2"}, "") ) def test_multiple_with_whitespace(self): @@ -334,7 +350,7 @@ def test_multiple_with_whitespace(self): resources.OTEL_RESOURCE_ATTRIBUTES ] = " k = v , k2 = v2 " self.assertEqual( - detector.detect(), resources.Resource({"k": "v", "k2": "v2"}) + detector.detect(), resources.Resource({"k": "v", "k2": "v2"}, "") ) @mock.patch.dict( @@ -345,7 +361,7 @@ def test_service_name_env(self): detector = resources.OTELResourceDetector() self.assertEqual( detector.detect(), - resources.Resource({"service.name": "test-srv-name"}), + resources.Resource({"service.name": "test-srv-name"}, ""), ) @mock.patch.dict( @@ -359,5 +375,5 @@ def test_service_name_env_precedence(self): detector = resources.OTELResourceDetector() self.assertEqual( detector.detect(), - resources.Resource({"service.name": "from-service-name"}), + resources.Resource({"service.name": "from-service-name"}, ""), ) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 50d8d2ae858..e2e55bf9d08 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -878,7 +878,7 @@ def test_span_override_start_and_end_time(self): self.assertEqual(end_time, span.end_time) def test_ended_span(self): - """"Events, attributes are not allowed after span is ended""" + """ "Events, attributes are not allowed after span is ended""" root = self.tracer.start_span("root") @@ -1229,9 +1229,9 @@ def test_to_json(self): is_remote=False, trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED), ) - parent = trace._Span("parent-name", context, resource=Resource({})) + parent = trace._Span("parent-name", context, resource=Resource({}, "")) span = trace._Span( - "span-name", context, resource=Resource({}), parent=parent + "span-name", context, resource=Resource({}, ""), parent=parent ) self.assertEqual( @@ -1268,7 +1268,7 @@ def test_attributes_to_json(self): is_remote=False, trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED), ) - span = trace._Span("span-name", context, resource=Resource({})) + span = trace._Span("span-name", context, resource=Resource({}, "")) span.set_attribute("key", "value") span.add_event("event", {"key2": "value2"}, 123) date_str = ns_to_iso_str(123) From db3cee07a9ee010a6bfa4c9592362bef01e5b17b Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Fri, 21 May 2021 20:16:05 -0700 Subject: [PATCH 04/32] Fix broken unrelated tests --- .../tests/test_jaeger_exporter_protobuf.py | 9 +++++---- .../tests/test_jaeger_exporter_thrift.py | 9 +++++---- .../tests/encoder/common_tests.py | 10 +++++----- .../tests/encoder/test_v1_json.py | 2 +- .../tests/encoder/test_v2_json.py | 2 +- .../tests/test_zipkin_exporter.py | 4 +++- .../tests/encoder/common_tests.py | 10 +++++----- .../tests/test_zipkin_exporter.py | 2 +- 8 files changed, 26 insertions(+), 22 deletions(-) diff --git a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py index 383e33d0f16..84192f68012 100644 --- a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py +++ b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py @@ -159,20 +159,20 @@ def test_translate_to_jaeger(self): links=(link,), kind=trace_api.SpanKind.CLIENT, resource=Resource( - attributes={"key_resource": "some_resource"} + attributes={"key_resource": "some_resource"}, schema_url="" ), ), trace._Span( name=span_names[1], context=parent_span_context, parent=None, - resource=Resource({}), + resource=Resource({}, ""), ), trace._Span( name=span_names[2], context=other_context, parent=None, - resource=Resource({}), + resource=Resource({}, ""), instrumentation_info=InstrumentationInfo( name="name", version="version" ), @@ -399,7 +399,8 @@ def test_max_tag_value_length(self): resource=Resource( attributes={ "key_resource": "some_resource some_resource some_more_resource" - } + }, + schema_url="", ), context=trace_api.SpanContext( trace_id=0x000000000000000000000000DEADBEEF, diff --git a/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py b/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py index 9c1773d4fad..8a0253f48ef 100644 --- a/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py +++ b/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py @@ -273,20 +273,20 @@ def test_translate_to_jaeger(self): links=(link,), kind=trace_api.SpanKind.CLIENT, resource=Resource( - attributes={"key_resource": "some_resource"} + attributes={"key_resource": "some_resource"}, schema_url="" ), ), trace._Span( name=span_names[1], context=parent_span_context, parent=None, - resource=Resource({}), + resource=Resource({}, ""), ), trace._Span( name=span_names[2], context=other_context, parent=None, - resource=Resource({}), + resource=Resource({}, ""), instrumentation_info=InstrumentationInfo( name="name", version="version" ), @@ -536,7 +536,8 @@ def test_max_tag_value_length(self): resource=Resource( attributes={ "key_resource": "some_resource some_resource some_more_resource" - } + }, + schema_url="", ), context=trace_api.SpanContext( trace_id=0x000000000000000000000000DEADBEEF, diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py index a04148b6ea4..ea7de796287 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py @@ -180,7 +180,7 @@ def get_data_for_max_tag_length_test( is_remote=False, trace_flags=TraceFlags(TraceFlags.SAMPLED), ), - resource=trace.Resource({}), + resource=trace.Resource({}, ""), ) span.start(start_time=start_time) span.set_attribute("string1", "v" * 500) @@ -379,7 +379,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, attributes={"key_bool": True} ), ), - resource=trace.Resource({}), + resource=trace.Resource({}, ""), ) span1.start(start_time=start_times[0]) span1.set_attribute("key_bool", False) @@ -393,7 +393,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=parent_span_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"} + attributes={"key_resource": "some_resource"}, schema_url="" ), ) span2.start(start_time=start_times[1]) @@ -405,7 +405,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"} + attributes={"key_resource": "some_resource"}, schema_url="" ), ) span3.start(start_time=start_times[2]) @@ -416,7 +416,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: name="test-span-3", context=other_context, parent=None, - resource=trace.Resource({}), + resource=trace.Resource({}, ""), instrumentation_info=InstrumentationInfo( name="name", version="version" ), diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py index c1956500bcf..e6e41fa024e 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py @@ -187,7 +187,7 @@ def test_encode_id_zero_padding(self): trace_flags=TraceFlags(TraceFlags.SAMPLED), ), parent=trace_api.SpanContext(trace_id, parent_id, is_remote=False), - resource=trace.Resource({}), + resource=trace.Resource({}, ""), ) otel_span.start(start_time=start_time) otel_span.end(end_time=end_time) diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py index eb0ad6000aa..e9717cda861 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py @@ -147,7 +147,7 @@ def test_encode_id_zero_padding(self): trace_flags=TraceFlags(TraceFlags.SAMPLED), ), parent=trace_api.SpanContext(trace_id, parent_id, is_remote=False), - resource=trace.Resource({}), + resource=trace.Resource({}, ""), ) otel_span.start(start_time=start_time) otel_span.end(end_time=end_time) diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py index 5c2aa0cbe69..9641597c0e4 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py @@ -44,7 +44,9 @@ class TestZipkinExporter(unittest.TestCase): def setUpClass(cls): trace.set_tracer_provider( TracerProvider( - resource=Resource({SERVICE_NAME: TEST_SERVICE_NAME}) + resource=Resource( + {SERVICE_NAME: TEST_SERVICE_NAME}, schema_url="" + ) ) ) diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py index a04148b6ea4..ea7de796287 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py @@ -180,7 +180,7 @@ def get_data_for_max_tag_length_test( is_remote=False, trace_flags=TraceFlags(TraceFlags.SAMPLED), ), - resource=trace.Resource({}), + resource=trace.Resource({}, ""), ) span.start(start_time=start_time) span.set_attribute("string1", "v" * 500) @@ -379,7 +379,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, attributes={"key_bool": True} ), ), - resource=trace.Resource({}), + resource=trace.Resource({}, ""), ) span1.start(start_time=start_times[0]) span1.set_attribute("key_bool", False) @@ -393,7 +393,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=parent_span_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"} + attributes={"key_resource": "some_resource"}, schema_url="" ), ) span2.start(start_time=start_times[1]) @@ -405,7 +405,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"} + attributes={"key_resource": "some_resource"}, schema_url="" ), ) span3.start(start_time=start_times[2]) @@ -416,7 +416,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: name="test-span-3", context=other_context, parent=None, - resource=trace.Resource({}), + resource=trace.Resource({}, ""), instrumentation_info=InstrumentationInfo( name="name", version="version" ), diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py index 8b8b01438e2..24c57b2d414 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py @@ -46,7 +46,7 @@ class TestZipkinExporter(unittest.TestCase): def setUpClass(cls): trace.set_tracer_provider( TracerProvider( - resource=Resource({SERVICE_NAME: TEST_SERVICE_NAME}) + resource=Resource({SERVICE_NAME: TEST_SERVICE_NAME}, "") ) ) From 6e1219d5d6570f414b7188fab536bfcd4ab7700c Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Fri, 21 May 2021 23:28:23 -0700 Subject: [PATCH 05/32] Add schema_url change to changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b9022d5f7d..a601696ec59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1854](https://github.com/open-telemetry/opentelemetry-python/pull/1854)) - Changed AttributeValue sequences to warn mypy users on adding None values to array ([#1855](https://github.com/open-telemetry/opentelemetry-python/pull/1855)) +- `Resource` now has a `schema_url` field. + ([#1862](https://github.com/open-telemetry/opentelemetry-python/issues/1862)) ## [1.2.0, 0.21b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.2.0-0.21b0) - 2021-05-11 From bac41bb5d4e4e4dcda8b46193b1f242296582e77 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Sat, 22 May 2021 00:00:50 -0700 Subject: [PATCH 06/32] Update documentation --- .../src/opentelemetry/sdk/resources/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 2d317f34e25..6cfdd7c94dc 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -154,6 +154,7 @@ def create( Args: attributes: Optional zero or more key-value pairs. + schema_url: Optional URL pointing to the schema Returns: The newly-created Resource. @@ -195,6 +196,10 @@ def merge(self, other: "Resource") -> "Resource": If a key exists on both the old and updating resource, the value of the updating resource will override the old resource value. + The new `schema_url` will take an updated value only if the original + `schema_url` is empty. Attempting to merge two resources with + different, non-empty values for `schema_url` will result in an error. + Args: other: The other resource to be merged. From 752611a8718dd0650e7ca7b5659c64644b8deeda Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Mon, 24 May 2021 11:43:39 -0700 Subject: [PATCH 07/32] Change changelog entry to match existing style Co-authored-by: Eddy Lin --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a601696ec59..1ed8f89e244 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#1854](https://github.com/open-telemetry/opentelemetry-python/pull/1854)) - Changed AttributeValue sequences to warn mypy users on adding None values to array ([#1855](https://github.com/open-telemetry/opentelemetry-python/pull/1855)) -- `Resource` now has a `schema_url` field. +- Added optional `schema_url` field to `Resource` class ([#1862](https://github.com/open-telemetry/opentelemetry-python/issues/1862)) ## [1.2.0, 0.21b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.2.0-0.21b0) - 2021-05-11 From 1b03a5c4202194fee0a7cf34d14d727d85f2ebda Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Mon, 24 May 2021 12:26:48 -0700 Subject: [PATCH 08/32] Remove assert statement --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 6cfdd7c94dc..ae28f6bcdad 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -142,7 +142,6 @@ class Resource: def __init__(self, attributes: Attributes, schema_url: str): self._attributes = attributes.copy() - assert schema_url is not None self._schema_url = schema_url @staticmethod From 8d7f8d1b45b9596c924f6829119d78e110bdb666 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Mon, 24 May 2021 12:27:53 -0700 Subject: [PATCH 09/32] Add Resource.schema_url to dunder eq and hash --- .../src/opentelemetry/sdk/resources/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index ae28f6bcdad..7b39c552847 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -224,10 +224,13 @@ def merge(self, other: "Resource") -> "Resource": def __eq__(self, other: object) -> bool: if not isinstance(other, Resource): return False - return self._attributes == other._attributes + return ( + self._attributes == other._attributes + and self._schema_url == other._schema_url + ) def __hash__(self): - return hash(dumps(self._attributes, sort_keys=True)) + return hash(dumps(self._attributes, sort_keys=True) + self._schema_url) _EMPTY_RESOURCE = Resource({}, "") From 99af3e7492a25eb675632ff9a298ed424c7003ff Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Mon, 24 May 2021 13:22:11 -0700 Subject: [PATCH 10/32] Change Resource.merge to log an error instead of throwing an exception --- .../src/opentelemetry/sdk/resources/__init__.py | 5 +++-- opentelemetry-sdk/tests/resources/test_resources.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 7b39c552847..dec35b6c17d 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -215,8 +215,9 @@ def merge(self, other: "Resource") -> "Resource": elif self.schema_url == other.schema_url: schema_url = other.schema_url else: - raise ValueError( - "The Schema URL of the old and updating resources are not empty and are different" + schema_url = "merge-conflict" + logger.error( + "Failed to merge resources: The Schema URL of the old and updating resources are not empty and are different" ) return Resource(merged_attributes, schema_url) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 032af4d6853..b31c84e71d6 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -17,6 +17,7 @@ import os import unittest from unittest import mock +from logging import ERROR from opentelemetry.sdk import resources @@ -143,7 +144,7 @@ def test_resource_merge(self): left = resources.Resource.create({}, schema_urls[0]) right = resources.Resource.create({}, schema_urls[1]) - with self.assertRaises(ValueError): + with self.assertLogs(level=ERROR): left.merge(right) def test_resource_merge_empty_string(self): From da1cbe2156a5a5c4287e85dae2dbe85677bedf01 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Mon, 24 May 2021 13:50:16 -0700 Subject: [PATCH 11/32] Change format of parameters for readability Co-authored-by: Eddy Lin --- .../tests/test_jaeger_exporter_protobuf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py index 84192f68012..da74d24f65b 100644 --- a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py +++ b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py @@ -159,7 +159,8 @@ def test_translate_to_jaeger(self): links=(link,), kind=trace_api.SpanKind.CLIENT, resource=Resource( - attributes={"key_resource": "some_resource"}, schema_url="" + attributes={"key_resource": "some_resource"}, + schema_url="", ), ), trace._Span( From d3afaff745dac977c5e1c75162d4f7596a9ff2ad Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Mon, 24 May 2021 14:07:20 -0700 Subject: [PATCH 12/32] Fix linting issues --- .../tests/test_jaeger_exporter_protobuf.py | 2 +- opentelemetry-sdk/tests/resources/test_resources.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py index da74d24f65b..6a3942cc82c 100644 --- a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py +++ b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py @@ -159,7 +159,7 @@ def test_translate_to_jaeger(self): links=(link,), kind=trace_api.SpanKind.CLIENT, resource=Resource( - attributes={"key_resource": "some_resource"}, + attributes={"key_resource": "some_resource"}, schema_url="", ), ), diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index b31c84e71d6..816b594d75c 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -16,8 +16,8 @@ import os import unittest -from unittest import mock from logging import ERROR +from unittest import mock from opentelemetry.sdk import resources From 602600f88ebff849369d32b445e2ca39ef2562ba Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 09:18:53 -0700 Subject: [PATCH 13/32] Fix merge --- opentelemetry-sdk/tests/resources/test_resources.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 1c14b76debd..2a326a947df 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -211,7 +211,8 @@ def test_invalid_resource_attribute_values(self): "": "empty-key-value", None: "null-key-value", "another-non-primitive": uuid.uuid4(), - } + }, + "", ) self.assertEqual( resource.attributes, From 168f24f702590bf060f5f4deedbfbabb1a4a8460 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 09:31:59 -0700 Subject: [PATCH 14/32] Typo Co-authored-by: Eddy Lin --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 5ecdd661899..06fa0c12a7b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -188,7 +188,7 @@ def attributes(self) -> Attributes: return self._attributes.copy() @property - def schema_url(self) -> Attributes: + def schema_url(self) -> str: return self._schema_url def merge(self, other: "Resource") -> "Resource": From 7a0c28613dfe094afe63f3bbd9ae72e26dc2af27 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 09:50:52 -0700 Subject: [PATCH 15/32] Add schema_url to immutability test --- opentelemetry-sdk/tests/resources/test_resources.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 2a326a947df..d199a41a8cb 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -184,7 +184,7 @@ def test_immutability(self): attributes_copy = attributes.copy() attributes_copy.update(default_attributes) - resource = resources.Resource.create(attributes) + resource = resources.Resource.create(attributes, "") self.assertEqual(resource.attributes, attributes_copy) resource.attributes["has_bugs"] = False @@ -193,6 +193,11 @@ def test_immutability(self): attributes["cost"] = 999.91 self.assertEqual(resource.attributes, attributes_copy) + with self.assertRaises(AttributeError): + resource.schema_url = "bug" + + self.assertEqual(resource.schema_url, "") + def test_service_name_using_process_name(self): resource = resources.Resource.create( {resources.PROCESS_EXECUTABLE_NAME: "test"} From b7450ebfadefa54c0076e2838273f23448ca838b Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 10:20:45 -0700 Subject: [PATCH 16/32] Implement small requested changes --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 2 +- opentelemetry-sdk/tests/resources/test_resources.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 5ecdd661899..55173748507 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -217,7 +217,7 @@ def merge(self, other: "Resource") -> "Resource": elif self.schema_url == other.schema_url: schema_url = other.schema_url else: - schema_url = "merge-conflict" + schema_url = "" logger.error( "Failed to merge resources: The Schema URL of the old and updating resources are not empty and are different" ) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index d199a41a8cb..76af6cb2695 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -52,6 +52,7 @@ def test_create(self): resource = resources.Resource.create(attributes) self.assertIsInstance(resource, resources.Resource) self.assertEqual(resource.attributes, expected_attributes) + self.assertEqual(resource.schema_url, "") schema_url = "https://opentelemetry.io/schemas/1.3.0" @@ -146,7 +147,7 @@ def test_resource_merge(self): left = resources.Resource.create({}, schema_urls[0]) right = resources.Resource.create({}, schema_urls[1]) with self.assertLogs(level=ERROR): - left.merge(right) + self.assertEqual(left.merge(right).schema_url, "") def test_resource_merge_empty_string(self): """Verify Resource.merge behavior with the empty string. From 00dac383b5dcf2679f6f47535a8d54c663fabcb8 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 10:29:39 -0700 Subject: [PATCH 17/32] Update Resource.merge to return an empty resource when an error occurs --- .../src/opentelemetry/sdk/resources/__init__.py | 7 ++++--- opentelemetry-sdk/tests/resources/test_resources.py | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 86be16643a2..9890862a7f5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -197,9 +197,10 @@ def merge(self, other: "Resource") -> "Resource": If a key exists on both the old and updating resource, the value of the updating resource will override the old resource value. - The new `schema_url` will take an updated value only if the original + The updating resource's `schema_url` will be used only if the old `schema_url` is empty. Attempting to merge two resources with - different, non-empty values for `schema_url` will result in an error. + different, non-empty values for `schema_url` will result in an error + and return an empty resource. Args: other: The other resource to be merged. @@ -217,10 +218,10 @@ def merge(self, other: "Resource") -> "Resource": elif self.schema_url == other.schema_url: schema_url = other.schema_url else: - schema_url = "" logger.error( "Failed to merge resources: The Schema URL of the old and updating resources are not empty and are different" ) + return _EMPTY_RESOURCE return Resource(merged_attributes, schema_url) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 76af6cb2695..6184493fc43 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -147,7 +147,7 @@ def test_resource_merge(self): left = resources.Resource.create({}, schema_urls[0]) right = resources.Resource.create({}, schema_urls[1]) with self.assertLogs(level=ERROR): - self.assertEqual(left.merge(right).schema_url, "") + self.assertEqual(left.merge(right), resources._EMPTY_RESOURCE) def test_resource_merge_empty_string(self): """Verify Resource.merge behavior with the empty string. From fca26140ac601a28b52929b27086612872e4685d Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 11:02:51 -0700 Subject: [PATCH 18/32] Add rejection of combined detectors with differing Schema URLs --- .../opentelemetry/sdk/resources/__init__.py | 9 +++++ .../tests/resources/test_resources.py | 35 +++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 9890862a7f5..ad6b5127227 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -296,6 +296,15 @@ def get_aggregated_resources( detector = detectors[detector_ind] try: detected_resources = future.result(timeout=timeout) + if ( + final_resource.schema_url != "" + and detected_resources.schema_url != "" + and final_resource != detected_resources + ): + logger.error( + "Failed to aggregate resources: The Schema URL of the detectors are not empty and are different" + ) + return _EMPTY_RESOURCE # pylint: disable=broad-except except Exception as ex: if detector.raise_on_error: diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 6184493fc43..42883fec6f3 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -296,6 +296,41 @@ def test_aggregated_resources_multiple_detectors(self): ), ) + def test_aggregated_resources_different_schema_urls(self): + resource_detector1 = mock.Mock(spec=resources.ResourceDetector) + resource_detector1.detect.return_value = resources.Resource( + {"key1": "value1"}, "" + ) + resource_detector2 = mock.Mock(spec=resources.ResourceDetector) + resource_detector2.detect.return_value = resources.Resource( + {"key2": "value2", "key3": "value3"}, "url1" + ) + resource_detector3 = mock.Mock(spec=resources.ResourceDetector) + resource_detector3.detect.return_value = resources.Resource( + { + "key2": "try_to_overwrite_existing_value", + "key3": "try_to_overwrite_existing_value", + "key4": "value4", + }, + "url2", + ) + self.assertEqual( + resources.get_aggregated_resources( + [resource_detector1, resource_detector2] + ), + resources.Resource( + {"key1": "value1", "key2": "value2", "key3": "value3"}, + "url1", + ), + ) + with self.assertLogs(level=ERROR): + self.assertEqual( + resources.get_aggregated_resources( + [resource_detector2, resource_detector3] + ), + resources._EMPTY_RESOURCE, + ) + def test_resource_detector_ignore_error(self): resource_detector = mock.Mock(spec=resources.ResourceDetector) resource_detector.detect.side_effect = Exception() From 5e1a5657adb8985d06c25a244c262aae141cc8a4 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 11:09:08 -0700 Subject: [PATCH 19/32] Indicate that Changelog is WIP --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b38676ca564..e341df21dbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Changed AttributeValue sequences to warn mypy users on adding None values to array ([#1855](https://github.com/open-telemetry/opentelemetry-python/pull/1855)) - Added optional `schema_url` field to `Resource` class - ([#1862](https://github.com/open-telemetry/opentelemetry-python/issues/1862)) + ([TBD](https://github.com/open-telemetry/opentelemetry-python/issues/1862)) ## [1.2.0, 0.21b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.2.0-0.21b0) - 2021-05-11 From 32ce84880e40d7f33b6736ba54affbf5a14085b6 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 11:12:31 -0700 Subject: [PATCH 20/32] Change Resource dunder hash to reduce collisions --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index ad6b5127227..4b52f5535a1 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -234,7 +234,9 @@ def __eq__(self, other: object) -> bool: ) def __hash__(self): - return hash(dumps(self._attributes, sort_keys=True) + self._schema_url) + return hash(dumps(self._attributes, sort_keys=True)) + 31 * hash( + self._schema_url + ) _EMPTY_RESOURCE = Resource({}, "") From 2801745f217728ea8522fd25ed9480b942c7fd4c Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 11:18:59 -0700 Subject: [PATCH 21/32] Change Resource dunder hash to reduce collisions Improves readability from the last commit --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 4b52f5535a1..bf504e9f7fe 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -234,8 +234,8 @@ def __eq__(self, other: object) -> bool: ) def __hash__(self): - return hash(dumps(self._attributes, sort_keys=True)) + 31 * hash( - self._schema_url + return hash( + dumps(self._attributes, sort_keys=True) + "|" + self._schema_url ) From 3debbeed970094e630f12ce58039d94c11b8d288 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 13:01:07 -0700 Subject: [PATCH 22/32] Revert "Change Resource dunder hash to reduce collisions" This reverts commit 2801745f217728ea8522fd25ed9480b942c7fd4c. --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index bf504e9f7fe..4b52f5535a1 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -234,8 +234,8 @@ def __eq__(self, other: object) -> bool: ) def __hash__(self): - return hash( - dumps(self._attributes, sort_keys=True) + "|" + self._schema_url + return hash(dumps(self._attributes, sort_keys=True)) + 31 * hash( + self._schema_url ) From f93845810c773cdc7f276b91e4cc6d99e9269bd4 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Tue, 25 May 2021 15:33:44 -0700 Subject: [PATCH 23/32] Update changelog with PR --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e341df21dbf..925f1464fcd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Changed AttributeValue sequences to warn mypy users on adding None values to array ([#1855](https://github.com/open-telemetry/opentelemetry-python/pull/1855)) - Added optional `schema_url` field to `Resource` class - ([TBD](https://github.com/open-telemetry/opentelemetry-python/issues/1862)) + ([#1871](https://github.com/open-telemetry/opentelemetry-python/pull/1871)) ## [1.2.0, 0.21b0](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.2.0-0.21b0) - 2021-05-11 From 1acf70215cfa138c292274cce3071d018404e191 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Wed, 26 May 2021 10:31:26 -0700 Subject: [PATCH 24/32] Change Resource constructor to take optional parameter --- .../tests/test_jaeger_exporter_protobuf.py | 10 ++-- .../tests/test_jaeger_exporter_thrift.py | 9 ++-- .../tests/test_otlp_trace_exporter.py | 2 +- .../tests/encoder/common_tests.py | 10 ++-- .../tests/encoder/test_v1_json.py | 2 +- .../tests/encoder/test_v2_json.py | 2 +- .../tests/test_zipkin_exporter.py | 4 +- .../tests/encoder/common_tests.py | 10 ++-- .../tests/test_zipkin_exporter.py | 2 +- .../opentelemetry/sdk/resources/__init__.py | 11 ++-- .../benchmarks/trace/test_benchmark_trace.py | 3 +- .../tests/resources/test_resources.py | 51 ++++++++----------- opentelemetry-sdk/tests/trace/test_trace.py | 6 +-- 13 files changed, 54 insertions(+), 68 deletions(-) diff --git a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py index 6a3942cc82c..383e33d0f16 100644 --- a/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py +++ b/exporter/opentelemetry-exporter-jaeger-proto-grpc/tests/test_jaeger_exporter_protobuf.py @@ -159,21 +159,20 @@ def test_translate_to_jaeger(self): links=(link,), kind=trace_api.SpanKind.CLIENT, resource=Resource( - attributes={"key_resource": "some_resource"}, - schema_url="", + attributes={"key_resource": "some_resource"} ), ), trace._Span( name=span_names[1], context=parent_span_context, parent=None, - resource=Resource({}, ""), + resource=Resource({}), ), trace._Span( name=span_names[2], context=other_context, parent=None, - resource=Resource({}, ""), + resource=Resource({}), instrumentation_info=InstrumentationInfo( name="name", version="version" ), @@ -400,8 +399,7 @@ def test_max_tag_value_length(self): resource=Resource( attributes={ "key_resource": "some_resource some_resource some_more_resource" - }, - schema_url="", + } ), context=trace_api.SpanContext( trace_id=0x000000000000000000000000DEADBEEF, diff --git a/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py b/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py index 8a0253f48ef..9c1773d4fad 100644 --- a/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py +++ b/exporter/opentelemetry-exporter-jaeger-thrift/tests/test_jaeger_exporter_thrift.py @@ -273,20 +273,20 @@ def test_translate_to_jaeger(self): links=(link,), kind=trace_api.SpanKind.CLIENT, resource=Resource( - attributes={"key_resource": "some_resource"}, schema_url="" + attributes={"key_resource": "some_resource"} ), ), trace._Span( name=span_names[1], context=parent_span_context, parent=None, - resource=Resource({}, ""), + resource=Resource({}), ), trace._Span( name=span_names[2], context=other_context, parent=None, - resource=Resource({}, ""), + resource=Resource({}), instrumentation_info=InstrumentationInfo( name="name", version="version" ), @@ -536,8 +536,7 @@ def test_max_tag_value_length(self): resource=Resource( attributes={ "key_resource": "some_resource some_resource some_more_resource" - }, - schema_url="", + } ), context=trace_api.SpanContext( trace_id=0x000000000000000000000000DEADBEEF, diff --git a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py index 9574196836d..3f43f1a8ccc 100644 --- a/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py +++ b/exporter/opentelemetry-exporter-otlp-proto-grpc/tests/test_otlp_trace_exporter.py @@ -149,7 +149,7 @@ def setUp(self): "trace_id": 67545097771067222548457157018666467027, } ), - resource=SDKResource(OrderedDict([("a", 1), ("b", False)]), ""), + resource=SDKResource(OrderedDict([("a", 1), ("b", False)])), parent=Mock(**{"span_id": 12345}), attributes=OrderedDict([("a", 1), ("b", True)]), events=[event_mock], diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py index ea7de796287..a04148b6ea4 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/common_tests.py @@ -180,7 +180,7 @@ def get_data_for_max_tag_length_test( is_remote=False, trace_flags=TraceFlags(TraceFlags.SAMPLED), ), - resource=trace.Resource({}, ""), + resource=trace.Resource({}), ) span.start(start_time=start_time) span.set_attribute("string1", "v" * 500) @@ -379,7 +379,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, attributes={"key_bool": True} ), ), - resource=trace.Resource({}, ""), + resource=trace.Resource({}), ) span1.start(start_time=start_times[0]) span1.set_attribute("key_bool", False) @@ -393,7 +393,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=parent_span_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"}, schema_url="" + attributes={"key_resource": "some_resource"} ), ) span2.start(start_time=start_times[1]) @@ -405,7 +405,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"}, schema_url="" + attributes={"key_resource": "some_resource"} ), ) span3.start(start_time=start_times[2]) @@ -416,7 +416,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: name="test-span-3", context=other_context, parent=None, - resource=trace.Resource({}, ""), + resource=trace.Resource({}), instrumentation_info=InstrumentationInfo( name="name", version="version" ), diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py index e6e41fa024e..c1956500bcf 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v1_json.py @@ -187,7 +187,7 @@ def test_encode_id_zero_padding(self): trace_flags=TraceFlags(TraceFlags.SAMPLED), ), parent=trace_api.SpanContext(trace_id, parent_id, is_remote=False), - resource=trace.Resource({}, ""), + resource=trace.Resource({}), ) otel_span.start(start_time=start_time) otel_span.end(end_time=end_time) diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py index e9717cda861..eb0ad6000aa 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/encoder/test_v2_json.py @@ -147,7 +147,7 @@ def test_encode_id_zero_padding(self): trace_flags=TraceFlags(TraceFlags.SAMPLED), ), parent=trace_api.SpanContext(trace_id, parent_id, is_remote=False), - resource=trace.Resource({}, ""), + resource=trace.Resource({}), ) otel_span.start(start_time=start_time) otel_span.end(end_time=end_time) diff --git a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py index 9641597c0e4..5c2aa0cbe69 100644 --- a/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-json/tests/test_zipkin_exporter.py @@ -44,9 +44,7 @@ class TestZipkinExporter(unittest.TestCase): def setUpClass(cls): trace.set_tracer_provider( TracerProvider( - resource=Resource( - {SERVICE_NAME: TEST_SERVICE_NAME}, schema_url="" - ) + resource=Resource({SERVICE_NAME: TEST_SERVICE_NAME}) ) ) diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py index ea7de796287..a04148b6ea4 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/encoder/common_tests.py @@ -180,7 +180,7 @@ def get_data_for_max_tag_length_test( is_remote=False, trace_flags=TraceFlags(TraceFlags.SAMPLED), ), - resource=trace.Resource({}, ""), + resource=trace.Resource({}), ) span.start(start_time=start_time) span.set_attribute("string1", "v" * 500) @@ -379,7 +379,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, attributes={"key_bool": True} ), ), - resource=trace.Resource({}, ""), + resource=trace.Resource({}), ) span1.start(start_time=start_times[0]) span1.set_attribute("key_bool", False) @@ -393,7 +393,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=parent_span_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"}, schema_url="" + attributes={"key_resource": "some_resource"} ), ) span2.start(start_time=start_times[1]) @@ -405,7 +405,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: context=other_context, parent=None, resource=trace.Resource( - attributes={"key_resource": "some_resource"}, schema_url="" + attributes={"key_resource": "some_resource"} ), ) span3.start(start_time=start_times[2]) @@ -416,7 +416,7 @@ def get_exhaustive_otel_span_list() -> List[trace._Span]: name="test-span-3", context=other_context, parent=None, - resource=trace.Resource({}, ""), + resource=trace.Resource({}), instrumentation_info=InstrumentationInfo( name="name", version="version" ), diff --git a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py index 24c57b2d414..8b8b01438e2 100644 --- a/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py +++ b/exporter/opentelemetry-exporter-zipkin-proto-http/tests/test_zipkin_exporter.py @@ -46,7 +46,7 @@ class TestZipkinExporter(unittest.TestCase): def setUpClass(cls): trace.set_tracer_provider( TracerProvider( - resource=Resource({SERVICE_NAME: TEST_SERVICE_NAME}, "") + resource=Resource({SERVICE_NAME: TEST_SERVICE_NAME}) ) ) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 4b52f5535a1..8965502f8a5 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -141,7 +141,9 @@ class Resource: """A Resource is an immutable representation of the entity producing telemetry as Attributes.""" - def __init__(self, attributes: Attributes, schema_url: str): + def __init__( + self, attributes: Attributes, schema_url: typing.Optional[str] = "" + ): _filter_attributes(attributes) self._attributes = attributes.copy() self._schema_url = schema_url @@ -239,14 +241,13 @@ def __hash__(self): ) -_EMPTY_RESOURCE = Resource({}, "") +_EMPTY_RESOURCE = Resource({}) _DEFAULT_RESOURCE = Resource( { TELEMETRY_SDK_LANGUAGE: "python", TELEMETRY_SDK_NAME: "opentelemetry", TELEMETRY_SDK_VERSION: _OPENTELEMETRY_SDK_VERSION, - }, - "", + } ) @@ -274,7 +275,7 @@ def detect(self) -> "Resource": service_name = os.environ.get(OTEL_SERVICE_NAME) if service_name: env_resource_map[SERVICE_NAME] = service_name - return Resource(env_resource_map, "") + return Resource(env_resource_map) def get_aggregated_resources( diff --git a/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py b/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py index c37a5409da8..a407a341f45 100644 --- a/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py +++ b/opentelemetry-sdk/tests/performance/benchmarks/trace/test_benchmark_trace.py @@ -23,8 +23,7 @@ "service.name": "A123456789", "service.version": "1.34567890", "service.instance.id": "123ab456-a123-12ab-12ab-12340a1abc12", - }, - "", + } ), ).get_tracer("sdk_tracer_provider") diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 42883fec6f3..1bf5c719dd7 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -117,11 +117,11 @@ def test_create(self): self.assertEqual(resource.schema_url, "") def test_resource_merge(self): - left = resources.Resource({"service": "ui"}, "") - right = resources.Resource({"host": "service-host"}, "") + left = resources.Resource({"service": "ui"}) + right = resources.Resource({"host": "service-host"}) self.assertEqual( left.merge(right), - resources.Resource({"service": "ui", "host": "service-host"}, ""), + resources.Resource({"service": "ui", "host": "service-host"}), ) schema_urls = ( "https://opentelemetry.io/schemas/1.2.0", @@ -156,15 +156,13 @@ def test_resource_merge_empty_string(self): the exception of the empty string. """ - left = resources.Resource({"service": "ui", "host": ""}, "") + left = resources.Resource({"service": "ui", "host": ""}) right = resources.Resource( - {"host": "service-host", "service": "not-ui"}, "" + {"host": "service-host", "service": "not-ui"} ) self.assertEqual( left.merge(right), - resources.Resource( - {"service": "not-ui", "host": "service-host"}, "" - ), + resources.Resource({"service": "not-ui", "host": "service-host"}), ) def test_immutability(self): @@ -185,7 +183,7 @@ def test_immutability(self): attributes_copy = attributes.copy() attributes_copy.update(default_attributes) - resource = resources.Resource.create(attributes, "") + resource = resources.Resource.create(attributes) self.assertEqual(resource.attributes, attributes_copy) resource.attributes["has_bugs"] = False @@ -217,8 +215,7 @@ def test_invalid_resource_attribute_values(self): "": "empty-key-value", None: "null-key-value", "another-non-primitive": uuid.uuid4(), - }, - "", + } ) self.assertEqual( resource.attributes, @@ -233,9 +230,7 @@ def test_aggregated_resources_no_detectors(self): self.assertEqual(aggregated_resources, resources.Resource.get_empty()) def test_aggregated_resources_with_static_resource(self): - static_resource = resources.Resource( - {"static_key": "static_value"}, "" - ) + static_resource = resources.Resource({"static_key": "static_value"}) self.assertEqual( resources.get_aggregated_resources( @@ -246,8 +241,7 @@ def test_aggregated_resources_with_static_resource(self): resource_detector = mock.Mock(spec=resources.ResourceDetector) resource_detector.detect.return_value = resources.Resource( - {"static_key": "try_to_overwrite_existing_value", "key": "value"}, - "", + {"static_key": "try_to_overwrite_existing_value", "key": "value"} ) self.assertEqual( resources.get_aggregated_resources( @@ -257,19 +251,18 @@ def test_aggregated_resources_with_static_resource(self): { "static_key": "try_to_overwrite_existing_value", "key": "value", - }, - "", + } ), ) def test_aggregated_resources_multiple_detectors(self): resource_detector1 = mock.Mock(spec=resources.ResourceDetector) resource_detector1.detect.return_value = resources.Resource( - {"key1": "value1"}, "" + {"key1": "value1"} ) resource_detector2 = mock.Mock(spec=resources.ResourceDetector) resource_detector2.detect.return_value = resources.Resource( - {"key2": "value2", "key3": "value3"}, "" + {"key2": "value2", "key3": "value3"} ) resource_detector3 = mock.Mock(spec=resources.ResourceDetector) resource_detector3.detect.return_value = resources.Resource( @@ -277,8 +270,7 @@ def test_aggregated_resources_multiple_detectors(self): "key2": "try_to_overwrite_existing_value", "key3": "try_to_overwrite_existing_value", "key4": "value4", - }, - "", + } ) self.assertEqual( @@ -291,8 +283,7 @@ def test_aggregated_resources_multiple_detectors(self): "key2": "try_to_overwrite_existing_value", "key3": "try_to_overwrite_existing_value", "key4": "value4", - }, - "", + } ), ) @@ -393,18 +384,18 @@ def test_empty(self): def test_one(self): detector = resources.OTELResourceDetector() os.environ[resources.OTEL_RESOURCE_ATTRIBUTES] = "k=v" - self.assertEqual(detector.detect(), resources.Resource({"k": "v"}, "")) + self.assertEqual(detector.detect(), resources.Resource({"k": "v"})) def test_one_with_whitespace(self): detector = resources.OTELResourceDetector() os.environ[resources.OTEL_RESOURCE_ATTRIBUTES] = " k = v " - self.assertEqual(detector.detect(), resources.Resource({"k": "v"}, "")) + self.assertEqual(detector.detect(), resources.Resource({"k": "v"})) def test_multiple(self): detector = resources.OTELResourceDetector() os.environ[resources.OTEL_RESOURCE_ATTRIBUTES] = "k=v,k2=v2" self.assertEqual( - detector.detect(), resources.Resource({"k": "v", "k2": "v2"}, "") + detector.detect(), resources.Resource({"k": "v", "k2": "v2"}) ) def test_multiple_with_whitespace(self): @@ -413,7 +404,7 @@ def test_multiple_with_whitespace(self): resources.OTEL_RESOURCE_ATTRIBUTES ] = " k = v , k2 = v2 " self.assertEqual( - detector.detect(), resources.Resource({"k": "v", "k2": "v2"}, "") + detector.detect(), resources.Resource({"k": "v", "k2": "v2"}) ) @mock.patch.dict( @@ -424,7 +415,7 @@ def test_service_name_env(self): detector = resources.OTELResourceDetector() self.assertEqual( detector.detect(), - resources.Resource({"service.name": "test-srv-name"}, ""), + resources.Resource({"service.name": "test-srv-name"}), ) @mock.patch.dict( @@ -438,5 +429,5 @@ def test_service_name_env_precedence(self): detector = resources.OTELResourceDetector() self.assertEqual( detector.detect(), - resources.Resource({"service.name": "from-service-name"}, ""), + resources.Resource({"service.name": "from-service-name"}), ) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index b9925409917..06b1fc1d6be 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -1220,9 +1220,9 @@ def test_to_json(self): is_remote=False, trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED), ) - parent = trace._Span("parent-name", context, resource=Resource({}, "")) + parent = trace._Span("parent-name", context, resource=Resource({})) span = trace._Span( - "span-name", context, resource=Resource({}, ""), parent=parent + "span-name", context, resource=Resource({}), parent=parent ) self.assertEqual( @@ -1259,7 +1259,7 @@ def test_attributes_to_json(self): is_remote=False, trace_flags=trace_api.TraceFlags(trace_api.TraceFlags.SAMPLED), ) - span = trace._Span("span-name", context, resource=Resource({}, "")) + span = trace._Span("span-name", context, resource=Resource({})) span.set_attribute("key", "value") span.add_event("event", {"key2": "value2"}, 123) date_str = ns_to_iso_str(123) From f7e628d6add48495886353a9439fed125f9e25d2 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Wed, 26 May 2021 13:37:47 -0700 Subject: [PATCH 25/32] Implement small changes --- .../src/opentelemetry/sdk/resources/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 8965502f8a5..c66d8cc0278 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -142,10 +142,12 @@ class Resource: """A Resource is an immutable representation of the entity producing telemetry as Attributes.""" def __init__( - self, attributes: Attributes, schema_url: typing.Optional[str] = "" + self, attributes: Attributes, schema_url: typing.Optional[str] = None ): _filter_attributes(attributes) self._attributes = attributes.copy() + if schema_url is None: + schema_url = "" self._schema_url = schema_url @staticmethod @@ -236,8 +238,8 @@ def __eq__(self, other: object) -> bool: ) def __hash__(self): - return hash(dumps(self._attributes, sort_keys=True)) + 31 * hash( - self._schema_url + return hash( + f"{dumps(self._attributes, sort_keys=True)}|{self._schema_url}" ) From b8361343e0122832d489f9f8c293882fa7ae3fe2 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Thu, 27 May 2021 10:30:11 -0700 Subject: [PATCH 26/32] Change Resource.merge to return old resource on error --- .../src/opentelemetry/sdk/resources/__init__.py | 4 ++-- opentelemetry-sdk/tests/resources/test_resources.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index c66d8cc0278..1f9f93e1e57 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -223,9 +223,9 @@ def merge(self, other: "Resource") -> "Resource": schema_url = other.schema_url else: logger.error( - "Failed to merge resources: The Schema URL of the old and updating resources are not empty and are different" + f"Failed to merge resources: The two schemas {self.schema_url} and {other.schema_url} are incompatible" ) - return _EMPTY_RESOURCE + return self return Resource(merged_attributes, schema_url) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 1bf5c719dd7..656635425f2 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -147,7 +147,7 @@ def test_resource_merge(self): left = resources.Resource.create({}, schema_urls[0]) right = resources.Resource.create({}, schema_urls[1]) with self.assertLogs(level=ERROR): - self.assertEqual(left.merge(right), resources._EMPTY_RESOURCE) + self.assertEqual(left.merge(right), left) def test_resource_merge_empty_string(self): """Verify Resource.merge behavior with the empty string. @@ -319,7 +319,9 @@ def test_aggregated_resources_different_schema_urls(self): resources.get_aggregated_resources( [resource_detector2, resource_detector3] ), - resources._EMPTY_RESOURCE, + resources.Resource( + {"key2": "value2", "key3": "value3"}, "url1" + ), ) def test_resource_detector_ignore_error(self): From 360dc736b299fde1e6a5e7cecebf5b798314bc8f Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Thu, 27 May 2021 10:33:34 -0700 Subject: [PATCH 27/32] Relegate ResourceDetector error checking to Resource.merge --- .../src/opentelemetry/sdk/resources/__init__.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 1f9f93e1e57..a0f69499e6b 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -301,15 +301,6 @@ def get_aggregated_resources( detector = detectors[detector_ind] try: detected_resources = future.result(timeout=timeout) - if ( - final_resource.schema_url != "" - and detected_resources.schema_url != "" - and final_resource != detected_resources - ): - logger.error( - "Failed to aggregate resources: The Schema URL of the detectors are not empty and are different" - ) - return _EMPTY_RESOURCE # pylint: disable=broad-except except Exception as ex: if detector.raise_on_error: From 6bbbc08f544c9aa1b5a5e810a0f316e12b663040 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Thu, 27 May 2021 10:51:28 -0700 Subject: [PATCH 28/32] Update tests for aggregating ResourceDetectors Adds a test to verify the behavior of aggregating ResourceDetectors with different schema URLs. Verifies that merge error logs contain both conflicting URLs. --- .../tests/resources/test_resources.py | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/tests/resources/test_resources.py b/opentelemetry-sdk/tests/resources/test_resources.py index 656635425f2..36eb099cd23 100644 --- a/opentelemetry-sdk/tests/resources/test_resources.py +++ b/opentelemetry-sdk/tests/resources/test_resources.py @@ -146,8 +146,10 @@ def test_resource_merge(self): left = resources.Resource.create({}, schema_urls[0]) right = resources.Resource.create({}, schema_urls[1]) - with self.assertLogs(level=ERROR): + with self.assertLogs(level=ERROR) as log_entry: self.assertEqual(left.merge(right), left) + self.assertIn(schema_urls[0], log_entry.output[0]) + self.assertIn(schema_urls[1], log_entry.output[0]) def test_resource_merge_empty_string(self): """Verify Resource.merge behavior with the empty string. @@ -305,6 +307,15 @@ def test_aggregated_resources_different_schema_urls(self): }, "url2", ) + resource_detector4 = mock.Mock(spec=resources.ResourceDetector) + resource_detector4.detect.return_value = resources.Resource( + { + "key2": "try_to_overwrite_existing_value", + "key3": "try_to_overwrite_existing_value", + "key4": "value4", + }, + "url1", + ) self.assertEqual( resources.get_aggregated_resources( [resource_detector1, resource_detector2] @@ -314,7 +325,7 @@ def test_aggregated_resources_different_schema_urls(self): "url1", ), ) - with self.assertLogs(level=ERROR): + with self.assertLogs(level=ERROR) as log_entry: self.assertEqual( resources.get_aggregated_resources( [resource_detector2, resource_detector3] @@ -323,6 +334,30 @@ def test_aggregated_resources_different_schema_urls(self): {"key2": "value2", "key3": "value3"}, "url1" ), ) + self.assertIn("url1", log_entry.output[0]) + self.assertIn("url2", log_entry.output[0]) + with self.assertLogs(level=ERROR): + self.assertEqual( + resources.get_aggregated_resources( + [ + resource_detector2, + resource_detector3, + resource_detector4, + resource_detector1, + ] + ), + resources.Resource( + { + "key1": "value1", + "key2": "try_to_overwrite_existing_value", + "key3": "try_to_overwrite_existing_value", + "key4": "value4", + }, + "url1", + ), + ) + self.assertIn("url1", log_entry.output[0]) + self.assertIn("url2", log_entry.output[0]) def test_resource_detector_ignore_error(self): resource_detector = mock.Mock(spec=resources.ResourceDetector) From da4bac1dc780c8cd60cbc5af46412306f247ef05 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Thu, 27 May 2021 11:01:03 -0700 Subject: [PATCH 29/32] Update Resource.merge comment --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index a0f69499e6b..bb78ee88f8e 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -204,7 +204,7 @@ def merge(self, other: "Resource") -> "Resource": The updating resource's `schema_url` will be used only if the old `schema_url` is empty. Attempting to merge two resources with different, non-empty values for `schema_url` will result in an error - and return an empty resource. + and return the old resource. Args: other: The other resource to be merged. From 09c1bf305c2256f9c630d2d49ac47cf24cb25a67 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Thu, 27 May 2021 11:10:35 -0700 Subject: [PATCH 30/32] Fix linting error --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index bb78ee88f8e..0abbf7fa26c 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -223,7 +223,9 @@ def merge(self, other: "Resource") -> "Resource": schema_url = other.schema_url else: logger.error( - f"Failed to merge resources: The two schemas {self.schema_url} and {other.schema_url} are incompatible" + "Failed to merge resources: The two schemas %s and %s are incompatible", + self.schema_url, + other.schema_url, ) return self From d218aadeae01fea10542f4bcca062d24c2a28ef7 Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Thu, 27 May 2021 16:54:54 -0700 Subject: [PATCH 31/32] Fix typo --- opentelemetry-sdk/tests/trace/test_trace.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/tests/trace/test_trace.py b/opentelemetry-sdk/tests/trace/test_trace.py index 06b1fc1d6be..a5113ffe098 100644 --- a/opentelemetry-sdk/tests/trace/test_trace.py +++ b/opentelemetry-sdk/tests/trace/test_trace.py @@ -869,7 +869,7 @@ def test_span_override_start_and_end_time(self): self.assertEqual(end_time, span.end_time) def test_ended_span(self): - """ "Events, attributes are not allowed after span is ended""" + """Events, attributes are not allowed after span is ended""" root = self.tracer.start_span("root") From e84eedba593c0cca26eeeb748bbba0f323c9e60c Mon Sep 17 00:00:00 2001 From: Daniel Getu Date: Thu, 27 May 2021 16:55:55 -0700 Subject: [PATCH 32/32] Remove redundant null check --- opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py index 0abbf7fa26c..24e5321ce90 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py @@ -166,8 +166,6 @@ def create( """ if not attributes: attributes = {} - if not schema_url: - schema_url = "" resource = _DEFAULT_RESOURCE.merge( OTELResourceDetector().detect() ).merge(Resource(attributes, schema_url))