From 38bd221042304335771158e48bb6c7ec6734b796 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 9 Sep 2026 09:31:11 -0300 Subject: [PATCH 1/5] fix(telemetry): merge SDK resource attrs into pre-installed MeterProvider When OTel auto-instrumentation installs a MeterProvider before auto_instrument() is called, metrics.set_meter_provider() is a no-op and the SDK resource attrs (sap.internal_sdk.version, sap.cloud_sdk.name) are never applied to metrics. Apply the same merge-on-existing pattern already used for LoggerProvider and TracerProvider: detect an SDK MeterProvider via isinstance, then mutate _sdk_config.resource in-place (SdkConfiguration is a dataclass; _measurement_consumer holds the same object reference so it sees the update automatically). --- src/sap_cloud_sdk/core/telemetry/_provider.py | 30 ++++++ tests/core/unit/telemetry/test_provider.py | 97 ++++++++++++++++--- 2 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 9c902971..367e2f21 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -30,6 +30,9 @@ ObservableUpDownCounter, UpDownCounter, ) + +# Stable reference for isinstance checks — not overwritten when tests patch MeterProvider +_SDKMeterProvider = MeterProvider from opentelemetry.sdk.metrics.export import ( AggregationTemporality, PeriodicExportingMetricReader, @@ -48,6 +51,18 @@ logger = logging.getLogger(__name__) +def _merge_sdk_resource_into_meter_provider( + provider: MeterProvider, sdk_resource: Resource +) -> None: + """OTel SDK has no public API to swap a MeterProvider's Resource after construction. + SdkConfiguration is a dataclass so _sdk_config.resource is mutable in-place; + _measurement_consumer holds the same object reference so it sees the update too.""" + provider._sdk_config.resource = provider._sdk_config.resource.merge(sdk_resource) + logger.info( + "Merged sap-cloud-sdk resource attrs onto wrapper-installed MeterProvider" + ) + + def _merge_sdk_resource_into_log_provider( provider: LoggerProvider, sdk_resource: Resource ) -> None: @@ -214,6 +229,21 @@ def _setup_meter_provider() -> Optional[MeterProvider]: try: resource = Resource.create(create_resource_attributes_from_env()) + existing = cast(MeterProvider, metrics.get_meter_provider()) + + if isinstance(existing, _SDKMeterProvider): + logger.warning( + "Global MeterProvider was already set by another library. " + "Merging sap.cloud_sdk.* resource attributes into the existing provider." + ) + _merge_sdk_resource_into_meter_provider(existing, resource) + logger.info( + f"OpenTelemetry meter provider merged. " + f"Service: {config.service_name}, " + f"Endpoint: {config.otlp_endpoint}" + ) + return existing + exporter = _create_metric_exporter() reader = PeriodicExportingMetricReader(exporter=exporter) provider = MeterProvider(resource=resource, metric_readers=[reader]) diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index 9118eff9..7ce15336 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -21,6 +21,7 @@ shutdown, _setup_meter_provider, _create_metric_exporter, + _merge_sdk_resource_into_meter_provider, setup_log_provider, _create_log_exporter, _merge_sdk_resource_into_log_provider, @@ -173,32 +174,66 @@ def test_delegates_to_create_metric_exporter(self): mock_exporter = MagicMock() with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): with patch("sap_cloud_sdk.core.telemetry._provider.Resource"): - with patch("sap_cloud_sdk.core.telemetry._provider._create_metric_exporter", return_value=mock_exporter) as mock_create: - with patch("sap_cloud_sdk.core.telemetry._provider.PeriodicExportingMetricReader") as mock_reader: - with patch("sap_cloud_sdk.core.telemetry._provider.MeterProvider"): - with patch("opentelemetry.metrics.set_meter_provider"): + with patch("sap_cloud_sdk.core.telemetry._provider.metrics") as mock_metrics: + mock_metrics.get_meter_provider.return_value = MagicMock() + with patch("sap_cloud_sdk.core.telemetry._provider._create_metric_exporter", return_value=mock_exporter) as mock_create: + with patch("sap_cloud_sdk.core.telemetry._provider.PeriodicExportingMetricReader") as mock_reader: + with patch("sap_cloud_sdk.core.telemetry._provider.MeterProvider"): _setup_meter_provider() - mock_create.assert_called_once_with() - mock_reader.assert_called_once_with(exporter=mock_exporter) + mock_create.assert_called_once_with() + mock_reader.assert_called_once_with(exporter=mock_exporter) def test_unsupported_protocol_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): with patch("sap_cloud_sdk.core.telemetry._provider.Resource"): - with patch.dict("os.environ", {"OTEL_EXPORTER_OTLP_PROTOCOL": "http/json"}): - assert _setup_meter_provider() is None + with patch("sap_cloud_sdk.core.telemetry._provider.metrics") as mock_metrics: + mock_metrics.get_meter_provider.return_value = MagicMock() + with patch.dict("os.environ", {"OTEL_EXPORTER_OTLP_PROTOCOL": "http/json"}): + assert _setup_meter_provider() is None def test_returns_configured_provider(self): mock_provider = MagicMock() with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): with patch("sap_cloud_sdk.core.telemetry._provider.Resource"): - with patch("sap_cloud_sdk.core.telemetry._provider._create_metric_exporter"): - with patch("sap_cloud_sdk.core.telemetry._provider.PeriodicExportingMetricReader"): - with patch("sap_cloud_sdk.core.telemetry._provider.MeterProvider", return_value=mock_provider): - with patch("opentelemetry.metrics.set_meter_provider"): + with patch("sap_cloud_sdk.core.telemetry._provider.metrics") as mock_metrics: + mock_metrics.get_meter_provider.return_value = MagicMock() + with patch("sap_cloud_sdk.core.telemetry._provider._create_metric_exporter"): + with patch("sap_cloud_sdk.core.telemetry._provider.PeriodicExportingMetricReader"): + with patch("sap_cloud_sdk.core.telemetry._provider.MeterProvider", return_value=mock_provider): assert _setup_meter_provider() is mock_provider + def test_reuses_existing_sdk_meter_provider(self): + """When a MeterProvider is already set, merge into it instead of creating a new one.""" + from opentelemetry.sdk.metrics import MeterProvider as _MP + existing = MagicMock(spec=_MP) + with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): + with patch("sap_cloud_sdk.core.telemetry._provider.Resource"): + with patch("sap_cloud_sdk.core.telemetry._provider.metrics") as mock_metrics: + mock_metrics.get_meter_provider.return_value = existing + with patch("sap_cloud_sdk.core.telemetry._provider._merge_sdk_resource_into_meter_provider") as mock_merge: + with patch("sap_cloud_sdk.core.telemetry._provider._SDKMeterProvider", _MP): + result = _setup_meter_provider() + assert result is existing + mock_merge.assert_called_once() + mock_metrics.set_meter_provider.assert_not_called() + + def test_existing_provider_no_new_reader(self): + """Reuse path must not create a new PeriodicExportingMetricReader.""" + from opentelemetry.sdk.metrics import MeterProvider as _MP + existing = MagicMock(spec=_MP) + with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): + with patch("sap_cloud_sdk.core.telemetry._provider.Resource"): + with patch("sap_cloud_sdk.core.telemetry._provider.metrics") as mock_metrics: + mock_metrics.get_meter_provider.return_value = existing + with patch("sap_cloud_sdk.core.telemetry._provider._merge_sdk_resource_into_meter_provider"): + with patch("sap_cloud_sdk.core.telemetry._provider._SDKMeterProvider", _MP): + with patch("sap_cloud_sdk.core.telemetry._provider.PeriodicExportingMetricReader") as mock_reader: + _setup_meter_provider() + mock_reader.assert_not_called() + + _LOGGING_HANDLER = "sap_cloud_sdk.core.telemetry._provider.LoggingHandler" _GRPC_LOG_EXPORTER = "sap_cloud_sdk.core.telemetry._provider.GRPCLogExporter" _HTTP_LOG_EXPORTER = "sap_cloud_sdk.core.telemetry._provider.HTTPLogExporter" @@ -330,6 +365,44 @@ def test_platform_path_adds_handler_when_none_present(self): mock_handler_cls.assert_called_once_with(logger_provider=external) +class TestMergeSdkResourceIntoMeterProvider: + def test_updates_sdk_config_resource(self): + from opentelemetry.sdk.metrics import MeterProvider as _MP + from opentelemetry.sdk.resources import Resource as _R + + sdk_resource = _R({"sap.cloud_sdk.language": "python"}) + existing_resource = _R({"service.name": "svc"}) + provider = _MP(resource=existing_resource) + + _merge_sdk_resource_into_meter_provider(provider, sdk_resource) + + assert provider._sdk_config.resource.attributes["sap.cloud_sdk.language"] == "python" + assert provider._sdk_config.resource.attributes["service.name"] == "svc" + + def test_measurement_consumer_sees_update(self): + """_measurement_consumer shares the same SdkConfiguration object.""" + from opentelemetry.sdk.metrics import MeterProvider as _MP + from opentelemetry.sdk.resources import Resource as _R + + sdk_resource = _R({"sap.cloud_sdk.language": "python"}) + provider = _MP(resource=_R({"service.name": "svc"})) + + _merge_sdk_resource_into_meter_provider(provider, sdk_resource) + + assert provider._measurement_consumer._sdk_config.resource is provider._sdk_config.resource + + def test_sdk_attrs_win_on_collision(self): + from opentelemetry.sdk.metrics import MeterProvider as _MP + from opentelemetry.sdk.resources import Resource as _R + + sdk_resource = _R({"service.name": "sdk-name"}) + provider = _MP(resource=_R({"service.name": "platform-name"})) + + _merge_sdk_resource_into_meter_provider(provider, sdk_resource) + + assert provider._sdk_config.resource.attributes["service.name"] == "sdk-name" + + class TestMergeSdkResourceIntoLogProvider: def test_updates_provider_resource(self): from opentelemetry.sdk._logs import LoggerProvider as _LP From 7764d755c37f3adaebc60dfbd2c72cc2a206e13d Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 9 Sep 2026 09:34:23 -0300 Subject: [PATCH 2/5] style: move _SDKMeterProvider alias after all imports --- src/sap_cloud_sdk/core/telemetry/_provider.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 367e2f21..f58d477a 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -30,15 +30,15 @@ ObservableUpDownCounter, UpDownCounter, ) - -# Stable reference for isinstance checks — not overwritten when tests patch MeterProvider -_SDKMeterProvider = MeterProvider from opentelemetry.sdk.metrics.export import ( AggregationTemporality, PeriodicExportingMetricReader, ) from opentelemetry.sdk.resources import Resource +# Stable reference for isinstance checks — not overwritten when tests patch MeterProvider +_SDKMeterProvider = MeterProvider + from sap_cloud_sdk.core.telemetry.config import ( get_config, create_resource_attributes_from_env, From 537359f278017a606748882db389f2b596b38805 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 9 Sep 2026 10:02:06 -0300 Subject: [PATCH 3/5] docs: explain why MeterProvider resource merge needs no lock --- src/sap_cloud_sdk/core/telemetry/_provider.py | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index f58d477a..6b0a7624 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -54,9 +54,23 @@ def _merge_sdk_resource_into_meter_provider( provider: MeterProvider, sdk_resource: Resource ) -> None: - """OTel SDK has no public API to swap a MeterProvider's Resource after construction. - SdkConfiguration is a dataclass so _sdk_config.resource is mutable in-place; - _measurement_consumer holds the same object reference so it sees the update too.""" + """Merge SDK resource attrs into an already-installed MeterProvider. + + OTel exposes no public API to swap a MeterProvider's Resource after + construction, so we mutate the private SdkConfiguration. This is safe and + needs no lock (unlike _merge_sdk_resource_into_log_provider): + + - SdkConfiguration is a dataclass; _sdk_config.resource is mutable in place. + - MetricReaderStorage.collect() reads _sdk_config.resource live at export + time via the same object reference (_measurement_consumer and every + MetricReaderStorage share it), so this single reassignment propagates. + - Meters do not cache their own resource, so there is no active-instances + collection to iterate under a lock. The log provider needs its + _active_loggers_lock only because each Logger caches its own _resource. + - The reassignment is a GIL-atomic single store, runs once at startup, and + Resource.merge preserves schema_url, so a race with the periodic collect + thread is negligible and self-heals on the next export cycle. + """ provider._sdk_config.resource = provider._sdk_config.resource.merge(sdk_resource) logger.info( "Merged sap-cloud-sdk resource attrs onto wrapper-installed MeterProvider" From 92ff2f05a63d4e772af618d740cf0bf6dd271b69 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 9 Sep 2026 10:06:02 -0300 Subject: [PATCH 4/5] docs: trim MeterProvider merge docstring --- src/sap_cloud_sdk/core/telemetry/_provider.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 6b0a7624..0a88ecdc 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -56,20 +56,10 @@ def _merge_sdk_resource_into_meter_provider( ) -> None: """Merge SDK resource attrs into an already-installed MeterProvider. - OTel exposes no public API to swap a MeterProvider's Resource after - construction, so we mutate the private SdkConfiguration. This is safe and - needs no lock (unlike _merge_sdk_resource_into_log_provider): - - - SdkConfiguration is a dataclass; _sdk_config.resource is mutable in place. - - MetricReaderStorage.collect() reads _sdk_config.resource live at export - time via the same object reference (_measurement_consumer and every - MetricReaderStorage share it), so this single reassignment propagates. - - Meters do not cache their own resource, so there is no active-instances - collection to iterate under a lock. The log provider needs its - _active_loggers_lock only because each Logger caches its own _resource. - - The reassignment is a GIL-atomic single store, runs once at startup, and - Resource.merge preserves schema_url, so a race with the periodic collect - thread is negligible and self-heals on the next export cycle. + No lock needed (unlike the log provider): meters don't cache a resource, so + there's no per-instance collection to iterate. collect() reads + _sdk_config.resource live at export time via the same shared reference we + mutate here, so this single reassignment propagates. """ provider._sdk_config.resource = provider._sdk_config.resource.merge(sdk_resource) logger.info( From 11ea20a14ea31fde6e547a3577f9d7c3fd1bf84b Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 9 Sep 2026 10:19:09 -0300 Subject: [PATCH 5/5] chore: version bump --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 459b7ff6..33906e97 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sap-cloud-sdk" -version = "0.51.1" +version = "0.51.2" description = "SAP Cloud SDK for Python" readme = "README.md" license = "Apache-2.0" diff --git a/uv.lock b/uv.lock index 3a4bed22..b3565574 100644 --- a/uv.lock +++ b/uv.lock @@ -4299,7 +4299,7 @@ wheels = [ [[package]] name = "sap-cloud-sdk" -version = "0.51.1" +version = "0.51.2" source = { editable = "." } dependencies = [ { name = "cryptography" },