From 78c19dcd764983be83d07faeca21abf3d2061a52 Mon Sep 17 00:00:00 2001 From: Vivek Khatri Date: Tue, 9 Jul 2024 22:23:29 +0530 Subject: [PATCH] Fix #3695: add attributes to get_meter fn and InstrumentationScope (#4015) --- CHANGELOG.md | 2 + .../metrics/_internal/__init__.py | 5 ++ .../sdk/metrics/_internal/__init__.py | 4 +- .../opentelemetry/sdk/util/instrumentation.py | 32 ++++++++-- .../tests/metrics/test_metrics.py | 64 +++++++++++++++++++ opentelemetry-sdk/tests/metrics/test_point.py | 4 +- 6 files changed, 104 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f2bc58afe..34f49c95b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3965](https://github.com/open-telemetry/opentelemetry-python/pull/3965)) - Validate links at span creation ([#3991](https://github.com/open-telemetry/opentelemetry-python/pull/3991)) +- Add attributes field in `MeterProvider.get_meter` and `InstrumentationScope` + ([#4015](https://github.com/open-telemetry/opentelemetry-python/pull/4015)) ## Version 1.25.0/0.46b0 (2024-05-30) diff --git a/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py b/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py index 9cbf14d2ed..972ff2707c 100644 --- a/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py +++ b/opentelemetry-api/src/opentelemetry/metrics/_internal/__init__.py @@ -75,6 +75,7 @@ ) from opentelemetry.util._once import Once from opentelemetry.util._providers import _load_provider +from opentelemetry.util.types import Attributes _logger = getLogger(__name__) @@ -102,6 +103,7 @@ def get_meter( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, + attributes: Optional[Attributes] = None, ) -> "Meter": """Returns a `Meter` for use by the given instrumentation library. @@ -128,6 +130,7 @@ def get_meter( ``importlib.metadata.version(instrumenting_library_name)``. schema_url: Optional. Specifies the Schema URL of the emitted telemetry. + attributes: Optional. Attributes that are associated with the emitted telemetry. """ @@ -139,6 +142,7 @@ def get_meter( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, + attributes: Optional[Attributes] = None, ) -> "Meter": """Returns a NoOpMeter.""" return NoOpMeter(name, version=version, schema_url=schema_url) @@ -155,6 +159,7 @@ def get_meter( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, + attributes: Optional[Attributes] = None, ) -> "Meter": with self._lock: if self._real_meter_provider is not None: diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py index 1e3a4528e3..9dc95c0edb 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/__init__.py @@ -54,6 +54,7 @@ from opentelemetry.sdk.resources import Resource from opentelemetry.sdk.util.instrumentation import InstrumentationScope from opentelemetry.util._once import Once +from opentelemetry.util.types import Attributes _logger = getLogger(__name__) @@ -518,6 +519,7 @@ def get_meter( name: str, version: Optional[str] = None, schema_url: Optional[str] = None, + attributes: Optional[Attributes] = None, ) -> Meter: if self._disabled: @@ -534,7 +536,7 @@ def get_meter( _logger.warning("Meter name cannot be None or empty.") return NoOpMeter(name, version=version, schema_url=schema_url) - info = InstrumentationScope(name, version, schema_url) + info = InstrumentationScope(name, version, schema_url, attributes) with self._meter_lock: if not self._meters.get(info): # FIXME #2558 pass SDKConfig object to meter so that the meter diff --git a/opentelemetry-sdk/src/opentelemetry/sdk/util/instrumentation.py b/opentelemetry-sdk/src/opentelemetry/sdk/util/instrumentation.py index 085d3fd874..a6fd7d7f66 100644 --- a/opentelemetry-sdk/src/opentelemetry/sdk/util/instrumentation.py +++ b/opentelemetry-sdk/src/opentelemetry/sdk/util/instrumentation.py @@ -16,6 +16,9 @@ from deprecated import deprecated +from opentelemetry.attributes import BoundedAttributes +from opentelemetry.util.types import Attributes + class InstrumentationInfo: """Immutable information about an instrumentation library module. @@ -82,22 +85,24 @@ class InstrumentationScope: properties. """ - __slots__ = ("_name", "_version", "_schema_url") + __slots__ = ("_name", "_version", "_schema_url", "_attributes") def __init__( self, name: str, version: Optional[str] = None, schema_url: Optional[str] = None, + attributes: Optional[Attributes] = None, ) -> None: self._name = name self._version = version if schema_url is None: schema_url = "" self._schema_url = schema_url + self._attributes = BoundedAttributes(attributes=attributes) def __repr__(self) -> str: - return f"{type(self).__name__}({self._name}, {self._version}, {self._schema_url})" + return f"{type(self).__name__}({self._name}, {self._version}, {self._schema_url}, {self._attributes})" def __hash__(self) -> int: return hash((self._name, self._version, self._schema_url)) @@ -105,19 +110,31 @@ def __hash__(self) -> int: def __eq__(self, value: object) -> bool: if not isinstance(value, InstrumentationScope): return NotImplemented - return (self._name, self._version, self._schema_url) == ( + return ( + self._name, + self._version, + self._schema_url, + self._attributes, + ) == ( value._name, value._version, value._schema_url, + value._attributes, ) def __lt__(self, value: object) -> bool: if not isinstance(value, InstrumentationScope): return NotImplemented - return (self._name, self._version, self._schema_url) < ( + return ( + self._name, + self._version, + self._schema_url, + self._attributes, + ) < ( value._name, value._version, value._schema_url, + value._attributes, ) @property @@ -132,12 +149,19 @@ def version(self) -> Optional[str]: def name(self) -> str: return self._name + @property + def attributes(self) -> Attributes: + return self._attributes + def to_json(self, indent=4) -> str: return dumps( { "name": self._name, "version": self._version, "schema_url": self._schema_url, + "attributes": ( + dict(self._attributes) if bool(self._attributes) else None + ), }, indent=indent, ) diff --git a/opentelemetry-sdk/tests/metrics/test_metrics.py b/opentelemetry-sdk/tests/metrics/test_metrics.py index 199305005b..d55262274b 100644 --- a/opentelemetry-sdk/tests/metrics/test_metrics.py +++ b/opentelemetry-sdk/tests/metrics/test_metrics.py @@ -18,6 +18,7 @@ from typing import Iterable, Sequence from unittest.mock import MagicMock, Mock, patch +from opentelemetry.attributes import BoundedAttributes from opentelemetry.metrics import NoOpMeter from opentelemetry.sdk.environment_variables import OTEL_SDK_DISABLED from opentelemetry.sdk.metrics import ( @@ -126,11 +127,36 @@ def test_get_meter(self): "name", version="version", schema_url="schema_url", + attributes={"key": "value"}, ) self.assertEqual(meter._instrumentation_scope.name, "name") self.assertEqual(meter._instrumentation_scope.version, "version") self.assertEqual(meter._instrumentation_scope.schema_url, "schema_url") + self.assertEqual( + meter._instrumentation_scope.attributes, {"key": "value"} + ) + + def test_get_meter_attributes(self): + """ + `MeterProvider.get_meter` arguments are used to create an + `InstrumentationScope` object on the created `Meter`. + """ + + meter = MeterProvider().get_meter( + "name", + version="version", + schema_url="schema_url", + attributes={"key": "value", "key2": 5, "key3": "value3"}, + ) + + self.assertEqual(meter._instrumentation_scope.name, "name") + self.assertEqual(meter._instrumentation_scope.version, "version") + self.assertEqual(meter._instrumentation_scope.schema_url, "schema_url") + self.assertEqual( + meter._instrumentation_scope.attributes, + {"key": "value", "key2": 5, "key3": "value3"}, + ) def test_get_meter_empty(self): """ @@ -180,6 +206,44 @@ def test_get_meter_duplicate(self): self.assertIs(meter1, meter2) self.assertIsNot(meter1, meter3) + def test_get_meter_comparison_with_attributes(self): + """ + Subsequent calls to `MeterProvider.get_meter` with the same arguments + should return the same `Meter` instance. + """ + mp = MeterProvider() + meter1 = mp.get_meter( + "name", + version="version", + schema_url="schema_url", + attributes={"key": "value", "key2": 5, "key3": "value3"}, + ) + meter2 = mp.get_meter( + "name", + version="version", + schema_url="schema_url", + attributes={"key": "value", "key2": 5, "key3": "value3"}, + ) + meter3 = mp.get_meter( + "name2", + version="version", + schema_url="schema_url", + ) + meter4 = mp.get_meter( + "name", + version="version", + schema_url="schema_url", + attributes={"key": "value", "key2": 5, "key3": "value4"}, + ) + self.assertIs(meter1, meter2) + self.assertIsNot(meter1, meter3) + self.assertTrue( + meter3._instrumentation_scope > meter4._instrumentation_scope + ) + self.assertIsInstance( + meter4._instrumentation_scope.attributes, BoundedAttributes + ) + def test_shutdown(self): mock_metric_reader_0 = MagicMock( diff --git a/opentelemetry-sdk/tests/metrics/test_point.py b/opentelemetry-sdk/tests/metrics/test_point.py index 20dd0e7238..cff07ff6ae 100644 --- a/opentelemetry-sdk/tests/metrics/test_point.py +++ b/opentelemetry-sdk/tests/metrics/test_point.py @@ -178,7 +178,7 @@ def setUpClass(cls): metrics=[cls.metric_0, cls.metric_1, cls.metric_2], schema_url="schema_url_0", ) - cls.scope_metrics_0_str = f'{{"scope": {{"name": "name_0", "version": "version_0", "schema_url": "schema_url_0"}}, "metrics": [{cls.metric_0_str}, {cls.metric_1_str}, {cls.metric_2_str}], "schema_url": "schema_url_0"}}' + cls.scope_metrics_0_str = f'{{"scope": {{"name": "name_0", "version": "version_0", "schema_url": "schema_url_0", "attributes": null}}, "metrics": [{cls.metric_0_str}, {cls.metric_1_str}, {cls.metric_2_str}], "schema_url": "schema_url_0"}}' cls.scope_metrics_1 = ScopeMetrics( scope=InstrumentationScope( @@ -189,7 +189,7 @@ def setUpClass(cls): metrics=[cls.metric_0, cls.metric_1, cls.metric_2], schema_url="schema_url_1", ) - cls.scope_metrics_1_str = f'{{"scope": {{"name": "name_1", "version": "version_1", "schema_url": "schema_url_1"}}, "metrics": [{cls.metric_0_str}, {cls.metric_1_str}, {cls.metric_2_str}], "schema_url": "schema_url_1"}}' + cls.scope_metrics_1_str = f'{{"scope": {{"name": "name_1", "version": "version_1", "schema_url": "schema_url_1", "attributes": null}}, "metrics": [{cls.metric_0_str}, {cls.metric_1_str}, {cls.metric_2_str}], "schema_url": "schema_url_1"}}' cls.resource_metrics_0 = ResourceMetrics( resource=Resource(