From 0528b12b489c3481ead2120dd6dc9b7bef8b5433 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Tue, 21 Jul 2026 16:14:32 -0300 Subject: [PATCH 01/15] feat(telemetry): add OTel standard logs as third signal Wire up a LoggerProvider alongside the existing metrics and traces so stdlib logging calls automatically flow through OTel with the shared resource attributes (service.name, region, subaccount, etc.). - setup_log_provider() in _provider.py mirrors _setup_meter_provider(): LoggerProvider + BatchLogRecordProcessor + OTLP exporter (grpc/http) - LoggingHandler from opentelemetry-instrumentation-logging installed on root logger so all existing logging.getLogger() calls are captured - auto_instrument() calls setup_log_provider() alongside trace setup, keeping the one-call contract for apps - user-guide.md: new Logging section with usage examples, structured fields, level filtering, and trace correlation notes - unit tests: mock-level coverage for exporter protocol, provider setup, handler installation, and auto_instrument wiring - e2e tests: real in-memory pipeline verifying body, severity, resource attributes, extra={} fields, and trace/span correlation --- src/sap_cloud_sdk/core/telemetry/_provider.py | 63 +++++++- .../core/telemetry/auto_instrument.py | 3 + .../core/telemetry/user-guide.md | 49 ++++++- .../unit/telemetry/test_auto_instrument.py | 27 ++++ .../unit/telemetry/test_log_provider_e2e.py | 137 ++++++++++++++++++ tests/core/unit/telemetry/test_provider.py | 109 +++++++++++++- 6 files changed, 385 insertions(+), 3 deletions(-) create mode 100644 tests/core/unit/telemetry/test_log_provider_e2e.py diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 420c3a46..af7379f6 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -1,16 +1,26 @@ -"""Internal module for setting up OpenTelemetry meter provider.""" +"""Internal module for setting up OpenTelemetry meter and logger providers.""" import logging import os from typing import Optional from opentelemetry import metrics +from opentelemetry._logs import set_logger_provider +from opentelemetry.exporter.otlp.proto.grpc._log_exporter import ( + OTLPLogExporter as GRPCLogExporter, +) from opentelemetry.exporter.otlp.proto.grpc.metric_exporter import ( OTLPMetricExporter as GRPCMetricExporter, ) +from opentelemetry.exporter.otlp.proto.http._log_exporter import ( + OTLPLogExporter as HTTPLogExporter, +) from opentelemetry.exporter.otlp.proto.http.metric_exporter import ( OTLPMetricExporter as HTTPMetricExporter, ) +from opentelemetry.instrumentation.logging.handler import LoggingHandler +from opentelemetry.sdk._logs import LoggerProvider +from opentelemetry.sdk._logs.export import BatchLogRecordProcessor from opentelemetry.sdk.metrics import ( MeterProvider, Counter, @@ -40,6 +50,9 @@ _meter_provider: Optional[MeterProvider] = None _meter: Optional[metrics.Meter] = None +# Global logger provider +_log_provider: Optional[LoggerProvider] = None + def get_meter() -> metrics.Meter: """Get or create the global meter instance. @@ -76,6 +89,54 @@ def shutdown() -> None: _meter_provider = None +def setup_log_provider() -> Optional[LoggerProvider]: + """Set up the global OTel LoggerProvider using the shared resource attributes. + + Installs a LoggingHandler on the root stdlib logger so all existing + logging.getLogger(...) calls in the app flow through OTel automatically. + No-op when telemetry is disabled. + """ + global _log_provider + + config = get_config() + if not config.enabled: + return None + + try: + resource = Resource.create(create_resource_attributes_from_env()) + exporter = _create_log_exporter() + provider = LoggerProvider(resource=resource) + provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) + set_logger_provider(provider) + + handler = LoggingHandler(logger_provider=provider) + logging.getLogger().addHandler(handler) + + _log_provider = provider + logger.info( + f"OpenTelemetry log provider initialized. " + f"Service: {config.service_name}, " + f"Endpoint: {config.otlp_endpoint}" + ) + return provider + + except Exception as e: + logger.error(f"Failed to initialize OpenTelemetry log provider: {e}") + return None + + +def _create_log_exporter(): + protocol = os.getenv(ENV_OTLP_PROTOCOL, "grpc").lower() + exporter_classes = {"grpc": GRPCLogExporter, "http/protobuf": HTTPLogExporter} + + if protocol not in exporter_classes: + raise ValueError( + f"Unsupported OTEL_EXPORTER_OTLP_PROTOCOL: '{protocol}'. " + "Supported values are 'grpc' and 'http/protobuf'." + ) + return exporter_classes[protocol]() + + def _create_metric_exporter(): protocol = os.getenv(ENV_OTLP_PROTOCOL, "grpc").lower() exporter_classes = {"grpc": GRPCMetricExporter, "http/protobuf": HTTPMetricExporter} diff --git a/src/sap_cloud_sdk/core/telemetry/auto_instrument.py b/src/sap_cloud_sdk/core/telemetry/auto_instrument.py index 7bd434a8..14f1ec3e 100644 --- a/src/sap_cloud_sdk/core/telemetry/auto_instrument.py +++ b/src/sap_cloud_sdk/core/telemetry/auto_instrument.py @@ -20,6 +20,7 @@ from opentelemetry.sdk.trace.export import ConsoleSpanExporter, SpanExporter from traceloop.sdk import Traceloop +from sap_cloud_sdk.core.telemetry._provider import setup_log_provider from sap_cloud_sdk.core.telemetry.module import Module from sap_cloud_sdk.core.telemetry.operation import Operation from sap_cloud_sdk.core.telemetry.config import ( @@ -97,6 +98,8 @@ def auto_instrument( _set_propagated_attributes_processor() _set_runtime_context_processor() + setup_log_provider() + if middlewares: _register_middleware_processors(middlewares) diff --git a/src/sap_cloud_sdk/core/telemetry/user-guide.md b/src/sap_cloud_sdk/core/telemetry/user-guide.md index c593920d..e2db8564 100644 --- a/src/sap_cloud_sdk/core/telemetry/user-guide.md +++ b/src/sap_cloud_sdk/core/telemetry/user-guide.md @@ -176,6 +176,48 @@ GenAIOperation.INVOKE_AGENT --- +## Logging + +`auto_instrument()` sets up OTel logs alongside traces and metrics. It installs a handler on the root stdlib logger so all existing `logging.getLogger(...)` calls in your app automatically ship log records to the OTel backend with the same resource attributes (service name, region, subaccount, etc.). + +No changes to your logging code are needed: + +```python +import logging + +logger = logging.getLogger(__name__) + +logger.info("Destination fetched") +logger.warning("Retrying request, attempt %d", attempt) +logger.error("Failed to connect", exc_info=True) +``` + +### Structured fields + +Use `extra={}` to attach structured attributes to a log record: + +```python +logger.info("Request completed", extra={"tenant_id": tid, "duration_ms": 120}) +``` + +### Log level filtering + +By default all levels (`DEBUG` and above) flow through OTel. To restrict what gets exported, set the level on the root logger or any specific logger: + +```python +# Only WARNING and above to OTel +logging.getLogger().setLevel(logging.WARNING) + +# Or scope it to your app's logger tree +logging.getLogger("my_app").setLevel(logging.INFO) +``` + +### Correlation with traces + +OTel logs emitted inside an active span are automatically correlated — the `trace_id` and `span_id` are injected into the log record. No extra work needed. + +--- + ## Adding attributes ### To the current span @@ -232,6 +274,7 @@ Propagation is scoped: once the parent span exits, its attributes stop propagati ## Complete example ```python +import logging from sap_cloud_sdk.core.telemetry import ( auto_instrument, invoke_agent_span, @@ -244,10 +287,13 @@ auto_instrument() from litellm import completion +logger = logging.getLogger(__name__) async def handle_request(query: str, user_id: str): set_tenant_id("bh7sjh...") + logger.info("Handling request", extra={"user_id": user_id}) + # Parent span carries business context for the whole agent turn. # Autoinstrumentation creates the child LLM span automatically. with invoke_agent_span( @@ -255,6 +301,7 @@ async def handle_request(query: str, user_id: str): ): documents = await retrieve_knowledge_base(query) add_span_attribute("documents.retrieved", len(documents)) + logger.debug("Retrieved %d documents", len(documents)) response = completion( model="gpt-4", @@ -345,7 +392,7 @@ export OTEL_EXPORTER_OTLP_ENDPOINT="https://otel-collector.example.com" ### Transport protocol -Both traces and metrics use gRPC by default. Switch to HTTP/protobuf by setting: +Traces, metrics, and logs all use gRPC by default. Switch to HTTP/protobuf by setting: ```bash export OTEL_EXPORTER_OTLP_PROTOCOL="http/protobuf" diff --git a/tests/core/unit/telemetry/test_auto_instrument.py b/tests/core/unit/telemetry/test_auto_instrument.py index a6d023bc..e95674b3 100644 --- a/tests/core/unit/telemetry/test_auto_instrument.py +++ b/tests/core/unit/telemetry/test_auto_instrument.py @@ -25,6 +25,7 @@ def mock_traceloop_components(): 'get_tracer_provider': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.trace.get_tracer_provider', return_value=create_autospec(SDKTracerProvider))), 'create_resource': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.create_resource_attributes_from_env')), 'get_app_name': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument._get_app_name')), + 'setup_log_provider': stack.enter_context(patch('sap_cloud_sdk.core.telemetry.auto_instrument.setup_log_provider')), } yield mocks @@ -405,3 +406,29 @@ def test_baggage_and_middleware_processors_both_added(self, mock_traceloop_compo # add_span_processor called 4 times: baggage, propagated attributes, runtime context, middleware assert mock_traceloop_components['get_tracer_provider'].return_value.add_span_processor.call_count == 4 + + +class TestAutoInstrumentLogging: + def test_setup_log_provider_called_on_instrument(self, mock_traceloop_components): + mock_traceloop_components['get_app_name'].return_value = 'test-app' + mock_traceloop_components['create_resource'].return_value = {} + + with patch.dict('os.environ', {'OTEL_EXPORTER_OTLP_ENDPOINT': 'http://localhost:4317'}, clear=True): + auto_instrument() + + mock_traceloop_components['setup_log_provider'].assert_called_once() + + def test_setup_log_provider_not_called_when_no_endpoint(self): + with patch.dict('os.environ', {}, clear=True): + with patch('sap_cloud_sdk.core.telemetry.auto_instrument.setup_log_provider') as mock_log: + auto_instrument() + mock_log.assert_not_called() + + def test_setup_log_provider_called_with_console_exporter(self, mock_traceloop_components): + mock_traceloop_components['get_app_name'].return_value = 'test-app' + mock_traceloop_components['create_resource'].return_value = {} + + with patch.dict('os.environ', {'OTEL_TRACES_EXPORTER': 'console'}, clear=True): + auto_instrument() + + mock_traceloop_components['setup_log_provider'].assert_called_once() diff --git a/tests/core/unit/telemetry/test_log_provider_e2e.py b/tests/core/unit/telemetry/test_log_provider_e2e.py new file mode 100644 index 00000000..36e3ed7d --- /dev/null +++ b/tests/core/unit/telemetry/test_log_provider_e2e.py @@ -0,0 +1,137 @@ +"""End-to-end tests for OTel log provider using a real in-memory exporter. + +These tests exercise the full log pipeline — LoggingHandler → LoggerProvider → +processor → exporter — without mocking. They verify that logs emitted via the +standard stdlib logging API arrive with the correct resource attributes and, +when inside an active span, carry trace/span correlation IDs. +""" + +import logging +import pytest + +from opentelemetry import trace +from opentelemetry._logs import _internal as _logs_internal +from opentelemetry.sdk._logs import LoggerProvider +from opentelemetry.sdk._logs.export import InMemoryLogRecordExporter, SimpleLogRecordProcessor +from opentelemetry.sdk.trace import TracerProvider +from opentelemetry.instrumentation.logging.handler import LoggingHandler + +from sap_cloud_sdk.core.telemetry._provider import setup_log_provider +from sap_cloud_sdk.core.telemetry.config import InstrumentationConfig + + +@pytest.fixture() +def log_exporter(monkeypatch): + """Set up a real LoggerProvider backed by an in-memory exporter. + + Resets the OTel logger provider singleton before and after each test so + tests are fully isolated. Uses SimpleLogRecordProcessor so records flush + synchronously without needing a flush() call. + """ + # Reset the OTel logger provider singleton so set_logger_provider() works + _logs_internal._LOGGER_PROVIDER_SET_ONCE._done = False + _logs_internal._LOGGER_PROVIDER = None + + exporter = InMemoryLogRecordExporter() + + monkeypatch.setattr( + "sap_cloud_sdk.core.telemetry._provider._create_log_exporter", + lambda: exporter, + ) + monkeypatch.setattr( + "sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor", + SimpleLogRecordProcessor, + ) + monkeypatch.setattr( + "sap_cloud_sdk.core.telemetry._provider.get_config", + lambda: InstrumentationConfig( + enabled=True, + service_name="test-svc", + otlp_endpoint="http://localhost:4317", + ), + ) + monkeypatch.setenv("APPFND_CONHOS_APP_NAME", "test-svc") + monkeypatch.setenv("APPFND_CONHOS_REGION", "eu10") + monkeypatch.setenv("APPFND_CONHOS_SUBACCOUNTID", "sub-123") + monkeypatch.setenv("APPFND_CONHOS_SYSTEM_ROLE", "TEST") + monkeypatch.setenv("SAP_SOLUTION_AREA", "AFND") + + provider = setup_log_provider() + assert provider is not None + + root = logging.getLogger() + original_level = root.level + root.setLevel(logging.DEBUG) + + yield exporter + + root.setLevel(original_level) + for h in list(root.handlers): + if isinstance(h, LoggingHandler): + root.removeHandler(h) + + # Reset singleton again so the next test starts clean + _logs_internal._LOGGER_PROVIDER_SET_ONCE._done = False + _logs_internal._LOGGER_PROVIDER = None + + +@pytest.fixture() +def tracer(): + provider = TracerProvider() + trace.set_tracer_provider(provider) + return provider.get_tracer("test") + + +class TestLogProviderEndToEnd: + def test_log_record_reaches_exporter(self, log_exporter): + logging.getLogger("test.basic").warning("hello from sdk") + + records = log_exporter.get_finished_logs() + assert len(records) == 1 + assert records[0].log_record.body == "hello from sdk" + + def test_severity_mapped_correctly(self, log_exporter): + logger = logging.getLogger("test.severity") + logger.info("info msg") + logger.warning("warn msg") + logger.error("error msg") + + records = log_exporter.get_finished_logs() + severities = [r.log_record.severity_text for r in records] + assert severities == ["INFO", "WARN", "ERROR"] + + def test_resource_attributes_on_record(self, log_exporter): + logging.getLogger("test.resource").info("check resource") + + r = log_exporter.get_finished_logs()[0] + attrs = r.resource.attributes + assert attrs.get("service.name") == "test-svc" + assert attrs.get("sap.cloud_sdk.language") == "python" + assert attrs.get("cloud.region") == "eu10" + assert attrs.get("sap.cld.subaccount_id") == "sub-123" + + def test_extra_fields_become_log_attributes(self, log_exporter): + logging.getLogger("test.extra").warning( + "structured log", extra={"tenant_id": "t-abc", "duration_ms": 42} + ) + + r = log_exporter.get_finished_logs()[0] + assert r.log_record.attributes.get("tenant_id") == "t-abc" + assert r.log_record.attributes.get("duration_ms") == 42 + + def test_trace_correlation_inside_span(self, log_exporter, tracer): + with tracer.start_as_current_span("test-span") as span: + logging.getLogger("test.trace").info("inside span") + expected_trace_id = span.get_span_context().trace_id + expected_span_id = span.get_span_context().span_id + + r = log_exporter.get_finished_logs()[0] + assert r.log_record.trace_id == expected_trace_id + assert r.log_record.span_id == expected_span_id + + def test_no_trace_correlation_outside_span(self, log_exporter): + logging.getLogger("test.no_trace").info("outside span") + + r = log_exporter.get_finished_logs()[0] + assert r.log_record.trace_id == 0 + assert r.log_record.span_id == 0 diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index 409e8880..90d47844 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -1,6 +1,7 @@ """Tests for telemetry meter provider.""" -from unittest.mock import patch, MagicMock +from unittest.mock import patch, MagicMock, call +import logging from opentelemetry.sdk.metrics import ( Counter, @@ -17,6 +18,8 @@ shutdown, _setup_meter_provider, _create_metric_exporter, + setup_log_provider, + _create_log_exporter, ) from sap_cloud_sdk.core.telemetry.config import InstrumentationConfig @@ -187,3 +190,107 @@ def test_returns_configured_provider(self): with patch("sap_cloud_sdk.core.telemetry._provider.MeterProvider", return_value=mock_provider): with patch("opentelemetry.metrics.set_meter_provider"): assert _setup_meter_provider() is mock_provider + + +_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" + + +class TestCreateLogExporter: + def test_grpc_by_default(self): + with patch(_GRPC_LOG_EXPORTER) as mock_grpc: + with patch(_HTTP_LOG_EXPORTER) as mock_http: + _create_log_exporter() + mock_grpc.assert_called_once_with() + mock_http.assert_not_called() + + def test_grpc_explicit(self): + with patch(_GRPC_LOG_EXPORTER) as mock_grpc: + with patch(_HTTP_LOG_EXPORTER) as mock_http: + with patch.dict("os.environ", {"OTEL_EXPORTER_OTLP_PROTOCOL": "grpc"}): + _create_log_exporter() + mock_grpc.assert_called_once_with() + mock_http.assert_not_called() + + def test_http_protobuf(self): + with patch(_GRPC_LOG_EXPORTER) as mock_grpc: + with patch(_HTTP_LOG_EXPORTER) as mock_http: + with patch.dict("os.environ", {"OTEL_EXPORTER_OTLP_PROTOCOL": "http/protobuf"}): + _create_log_exporter() + mock_http.assert_called_once_with() + mock_grpc.assert_not_called() + + def test_unsupported_protocol_raises(self): + import pytest + with patch.dict("os.environ", {"OTEL_EXPORTER_OTLP_PROTOCOL": "http/json"}): + with pytest.raises(ValueError, match="Unsupported OTEL_EXPORTER_OTLP_PROTOCOL"): + _create_log_exporter() + + +class TestSetupLogProvider: + def test_disabled_returns_none(self): + config = InstrumentationConfig(enabled=False) + with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=config): + assert setup_log_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_log_exporter"): + with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): + with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=mock_provider): + with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): + with patch(_LOGGING_HANDLER): + with patch("logging.getLogger"): + result = setup_log_provider() + assert result is mock_provider + + def test_uses_shared_resource_attributes(self): + with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): + with patch("sap_cloud_sdk.core.telemetry._provider.create_resource_attributes_from_env", return_value={"service.name": "svc"}) as mock_attrs: + with patch("sap_cloud_sdk.core.telemetry._provider.Resource") as mock_resource: + with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter"): + with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): + with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider"): + with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): + with patch(_LOGGING_HANDLER): + with patch("logging.getLogger"): + setup_log_provider() + mock_attrs.assert_called_once() + mock_resource.create.assert_called_once_with({"service.name": "svc"}) + + def test_calls_set_logger_provider(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("sap_cloud_sdk.core.telemetry._provider._create_log_exporter"): + with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): + mock_provider = MagicMock() + with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=mock_provider): + with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider") as mock_set: + with patch(_LOGGING_HANDLER): + with patch("logging.getLogger"): + setup_log_provider() + mock_set.assert_called_once_with(mock_provider) + + def test_installs_handler_on_root_logger(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("sap_cloud_sdk.core.telemetry._provider._create_log_exporter"): + with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): + with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider"): + with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): + mock_handler = MagicMock() + with patch(_LOGGING_HANDLER, return_value=mock_handler): + mock_root = MagicMock() + with patch("logging.getLogger", return_value=mock_root) as mock_get_logger: + setup_log_provider() + mock_get_logger.assert_called_once_with() + mock_root.addHandler.assert_called_once_with(mock_handler) + + def test_exception_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("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", side_effect=Exception("boom")): + assert setup_log_provider() is None From 5bfe5317898b6d354397d641183a92d497e080b1 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 22 Jul 2026 09:29:10 -0300 Subject: [PATCH 02/15] docs(telemetry): document third-party logging library compatibility --- src/sap_cloud_sdk/core/telemetry/user-guide.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/sap_cloud_sdk/core/telemetry/user-guide.md b/src/sap_cloud_sdk/core/telemetry/user-guide.md index e2db8564..4af91c47 100644 --- a/src/sap_cloud_sdk/core/telemetry/user-guide.md +++ b/src/sap_cloud_sdk/core/telemetry/user-guide.md @@ -216,6 +216,12 @@ logging.getLogger("my_app").setLevel(logging.INFO) OTel logs emitted inside an active span are automatically correlated — the `trace_id` and `span_id` are injected into the log record. No extra work needed. +### Third-party logging libraries + +The OTel handler is installed on the root stdlib `logging` logger. Any library that propagates to stdlib works automatically. + +Libraries that bypass stdlib entirely need a custom sink that forwards records to `logging.getLogger(...).log(...)`. The OTel handler then picks them up from there. + --- ## Adding attributes From be9dd9370c12904dd12a37aa6a91456d6862cb74 Mon Sep 17 00:00:00 2001 From: Lucas Soares Date: Tue, 28 Jul 2026 17:43:01 +0200 Subject: [PATCH 03/15] idempotency guard --- src/sap_cloud_sdk/core/telemetry/_provider.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index af7379f6..6c403155 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -98,6 +98,9 @@ def setup_log_provider() -> Optional[LoggerProvider]: """ global _log_provider + if _log_provider is not None: + return _log_provider + config = get_config() if not config.enabled: return None From 8eac3d4e2c6c35d3a3e13aeffd444a335bd3cba0 Mon Sep 17 00:00:00 2001 From: Lucas Soares Date: Tue, 28 Jul 2026 17:58:59 +0200 Subject: [PATCH 04/15] Unit tests --- tests/core/unit/telemetry/test_log_provider_e2e.py | 4 ++++ tests/core/unit/telemetry/test_provider.py | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/tests/core/unit/telemetry/test_log_provider_e2e.py b/tests/core/unit/telemetry/test_log_provider_e2e.py index 36e3ed7d..c7377d4e 100644 --- a/tests/core/unit/telemetry/test_log_provider_e2e.py +++ b/tests/core/unit/telemetry/test_log_provider_e2e.py @@ -32,6 +32,9 @@ def log_exporter(monkeypatch): _logs_internal._LOGGER_PROVIDER_SET_ONCE._done = False _logs_internal._LOGGER_PROVIDER = None + import sap_cloud_sdk.core.telemetry._provider as provider_module + provider_module._log_provider = None + exporter = InMemoryLogRecordExporter() monkeypatch.setattr( @@ -73,6 +76,7 @@ def log_exporter(monkeypatch): # Reset singleton again so the next test starts clean _logs_internal._LOGGER_PROVIDER_SET_ONCE._done = False _logs_internal._LOGGER_PROVIDER = None + provider_module._log_provider = None @pytest.fixture() diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index 90d47844..d06869d6 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -2,6 +2,7 @@ from unittest.mock import patch, MagicMock, call import logging +import pytest from opentelemetry.sdk.metrics import ( Counter, @@ -229,6 +230,13 @@ def test_unsupported_protocol_raises(self): class TestSetupLogProvider: + @pytest.fixture(autouse=True) + def reset_log_provider(self): + import sap_cloud_sdk.core.telemetry._provider as provider_module + provider_module._log_provider = None + yield + provider_module._log_provider = None + def test_disabled_returns_none(self): config = InstrumentationConfig(enabled=False) with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=config): From 2980d47ad42dd60d24430db7b57e06f711d99d89 Mon Sep 17 00:00:00 2001 From: Lucas Soares Date: Tue, 28 Jul 2026 18:14:46 +0200 Subject: [PATCH 05/15] Unit tests --- tests/core/unit/telemetry/test_log_provider_e2e.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/core/unit/telemetry/test_log_provider_e2e.py b/tests/core/unit/telemetry/test_log_provider_e2e.py index c7377d4e..efc65865 100644 --- a/tests/core/unit/telemetry/test_log_provider_e2e.py +++ b/tests/core/unit/telemetry/test_log_provider_e2e.py @@ -35,6 +35,12 @@ def log_exporter(monkeypatch): import sap_cloud_sdk.core.telemetry._provider as provider_module provider_module._log_provider = None + # Remove any stale LoggingHandlers left by previous tests + root = logging.getLogger() + for h in list(root.handlers): + if isinstance(h, LoggingHandler): + root.removeHandler(h) + exporter = InMemoryLogRecordExporter() monkeypatch.setattr( From 5f51ccf01879210a3c4fac3818056c046c136177 Mon Sep 17 00:00:00 2001 From: Lucas Soares Date: Tue, 28 Jul 2026 18:38:14 +0200 Subject: [PATCH 06/15] Attach logger provider if not exists --- src/sap_cloud_sdk/core/telemetry/_provider.py | 12 ++++++--- tests/core/unit/telemetry/test_provider.py | 27 ++++++++++--------- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 6c403155..19338a3f 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -5,7 +5,7 @@ from typing import Optional from opentelemetry import metrics -from opentelemetry._logs import set_logger_provider +from opentelemetry._logs import get_logger_provider, set_logger_provider from opentelemetry.exporter.otlp.proto.grpc._log_exporter import ( OTLPLogExporter as GRPCLogExporter, ) @@ -108,9 +108,13 @@ def setup_log_provider() -> Optional[LoggerProvider]: try: resource = Resource.create(create_resource_attributes_from_env()) exporter = _create_log_exporter() - provider = LoggerProvider(resource=resource) - provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) - set_logger_provider(provider) + candidate = LoggerProvider(resource=resource) + candidate.add_log_record_processor(BatchLogRecordProcessor(exporter)) + set_logger_provider(candidate) + + provider = get_logger_provider() + if provider is not candidate: + provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) handler = LoggingHandler(logger_provider=provider) logging.getLogger().addHandler(handler) diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index d06869d6..534ee5de 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -250,10 +250,11 @@ def test_returns_configured_provider(self): with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=mock_provider): with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): - with patch(_LOGGING_HANDLER): - with patch("logging.getLogger"): - result = setup_log_provider() - assert result is mock_provider + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=mock_provider): + with patch(_LOGGING_HANDLER): + with patch("logging.getLogger"): + result = setup_log_provider() + assert result is mock_provider def test_uses_shared_resource_attributes(self): with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): @@ -287,15 +288,17 @@ def test_installs_handler_on_root_logger(self): with patch("sap_cloud_sdk.core.telemetry._provider.Resource"): with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter"): with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): - with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider"): + mock_provider = MagicMock() + with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=mock_provider): with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): - mock_handler = MagicMock() - with patch(_LOGGING_HANDLER, return_value=mock_handler): - mock_root = MagicMock() - with patch("logging.getLogger", return_value=mock_root) as mock_get_logger: - setup_log_provider() - mock_get_logger.assert_called_once_with() - mock_root.addHandler.assert_called_once_with(mock_handler) + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=mock_provider): + mock_handler = MagicMock() + with patch(_LOGGING_HANDLER, return_value=mock_handler): + mock_root = MagicMock() + with patch("logging.getLogger", return_value=mock_root) as mock_get_logger: + setup_log_provider() + mock_get_logger.assert_called_once_with() + mock_root.addHandler.assert_called_once_with(mock_handler) def test_exception_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): From 98c799697924fa6a0463409a1e4eb15eea07a242 Mon Sep 17 00:00:00 2001 From: Lucas Soares Date: Thu, 30 Jul 2026 11:30:52 +0200 Subject: [PATCH 07/15] clashing providers --- src/sap_cloud_sdk/core/telemetry/_provider.py | 2 -- tests/core/unit/telemetry/test_provider.py | 21 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 19338a3f..d93af041 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -113,8 +113,6 @@ def setup_log_provider() -> Optional[LoggerProvider]: set_logger_provider(candidate) provider = get_logger_provider() - if provider is not candidate: - provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) handler = LoggingHandler(logger_provider=provider) logging.getLogger().addHandler(handler) diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index 534ee5de..ac99e1d8 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -305,3 +305,24 @@ def test_exception_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider.Resource"): with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", side_effect=Exception("boom")): assert setup_log_provider() is None + + def test_external_provider_wins_no_second_exporter(self): + external_provider = MagicMock() + candidate = 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_log_exporter"): + with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor") as mock_proc: + with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=candidate): + with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=external_provider): + with patch(_LOGGING_HANDLER) as mock_handler_cls: + with patch("logging.getLogger"): + result = setup_log_provider() + + # returns the external provider, not our candidate + assert result is external_provider + # BatchLogRecordProcessor only called once (for candidate setup) + mock_proc.assert_called_once() + # handler wired to external provider + mock_handler_cls.assert_called_once_with(logger_provider=external_provider) From 9e150dd409727724f9b053765f56ac79d328bf8d Mon Sep 17 00:00:00 2001 From: Lucas Soares Date: Thu, 30 Jul 2026 12:51:19 +0200 Subject: [PATCH 08/15] attaching log record processor --- src/sap_cloud_sdk/core/telemetry/_provider.py | 7 +++ .../unit/telemetry/test_log_provider_e2e.py | 62 +++++++++++++++++++ tests/core/unit/telemetry/test_provider.py | 8 ++- 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index d93af041..564e27de 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -114,6 +114,13 @@ def setup_log_provider() -> Optional[LoggerProvider]: provider = get_logger_provider() + if provider is not candidate: + logger.warning( + "Global LoggerProvider was already set by another library. " + "Attaching SAP log processor to the existing provider." + ) + provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) + handler = LoggingHandler(logger_provider=provider) logging.getLogger().addHandler(handler) diff --git a/tests/core/unit/telemetry/test_log_provider_e2e.py b/tests/core/unit/telemetry/test_log_provider_e2e.py index efc65865..0b14aa14 100644 --- a/tests/core/unit/telemetry/test_log_provider_e2e.py +++ b/tests/core/unit/telemetry/test_log_provider_e2e.py @@ -145,3 +145,65 @@ def test_no_trace_correlation_outside_span(self, log_exporter): r = log_exporter.get_finished_logs()[0] assert r.log_record.trace_id == 0 assert r.log_record.span_id == 0 + + +class TestLogProviderClashingProvider: + """When another library claims the global LoggerProvider first, our processor + must still be attached so logs reach the SAP OTLP endpoint.""" + + def test_logs_reach_our_exporter_when_provider_already_set(self, monkeypatch): + from opentelemetry._logs import _internal as _logs_internal + from opentelemetry.sdk._logs.export import InMemoryLogRecordExporter, SimpleLogRecordProcessor + import sap_cloud_sdk.core.telemetry._provider as provider_module + + # Reset OTel singleton + _logs_internal._LOGGER_PROVIDER_SET_ONCE._done = False + _logs_internal._LOGGER_PROVIDER = None + provider_module._log_provider = None + + # Remove stale LoggingHandlers + root = logging.getLogger() + for h in list(root.handlers): + if isinstance(h, LoggingHandler): + root.removeHandler(h) + + # Simulate another library claiming the provider first + external_exporter = InMemoryLogRecordExporter() + external_provider = LoggerProvider() + external_provider.add_log_record_processor(SimpleLogRecordProcessor(external_exporter)) + from opentelemetry._logs import set_logger_provider + set_logger_provider(external_provider) + + # Now our SDK runs setup_log_provider + our_exporter = InMemoryLogRecordExporter() + monkeypatch.setattr("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", lambda: our_exporter) + monkeypatch.setattr("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor", SimpleLogRecordProcessor) + monkeypatch.setattr( + "sap_cloud_sdk.core.telemetry._provider.get_config", + lambda: __import__( + "sap_cloud_sdk.core.telemetry.config", fromlist=["InstrumentationConfig"] + ).InstrumentationConfig(enabled=True, service_name="test-svc", otlp_endpoint="http://localhost:4317"), + ) + monkeypatch.setenv("APPFND_CONHOS_APP_NAME", "test-svc") + monkeypatch.setenv("APPFND_CONHOS_REGION", "eu10") + monkeypatch.setenv("APPFND_CONHOS_SUBACCOUNTID", "sub-123") + monkeypatch.setenv("APPFND_CONHOS_SYSTEM_ROLE", "TEST") + monkeypatch.setenv("SAP_SOLUTION_AREA", "AFND") + + provider = setup_log_provider() + assert provider is external_provider + + root.setLevel(logging.DEBUG) + logging.getLogger("test.clash").warning("hello from sdk") + + our_records = our_exporter.get_finished_logs() + assert len(our_records) == 1 + assert our_records[0].log_record.body == "hello from sdk" + + # Cleanup + for h in list(root.handlers): + if isinstance(h, LoggingHandler): + root.removeHandler(h) + _logs_internal._LOGGER_PROVIDER_SET_ONCE._done = False + _logs_internal._LOGGER_PROVIDER = None + provider_module._log_provider = None diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index ac99e1d8..cddbeb3f 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -306,7 +306,7 @@ def test_exception_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", side_effect=Exception("boom")): assert setup_log_provider() is None - def test_external_provider_wins_no_second_exporter(self): + def test_external_provider_gets_our_processor_attached(self): external_provider = MagicMock() candidate = MagicMock() with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): @@ -322,7 +322,9 @@ def test_external_provider_wins_no_second_exporter(self): # returns the external provider, not our candidate assert result is external_provider - # BatchLogRecordProcessor only called once (for candidate setup) - mock_proc.assert_called_once() + # BatchLogRecordProcessor called twice: once for our candidate, + # once to attach our processor to the external provider + assert mock_proc.call_count == 2 + external_provider.add_log_record_processor.assert_called_once() # handler wired to external provider mock_handler_cls.assert_called_once_with(logger_provider=external_provider) From b578f78a3063fe7eeb84ff62796dc5eb7e4c8b03 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Tue, 4 Aug 2026 18:13:50 -0700 Subject: [PATCH 09/15] fix(telemetry): cast get_logger_provider() to SDK type to satisfy ty checker --- src/sap_cloud_sdk/core/telemetry/_provider.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 564e27de..9ee149c8 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -2,7 +2,7 @@ import logging import os -from typing import Optional +from typing import Optional, cast from opentelemetry import metrics from opentelemetry._logs import get_logger_provider, set_logger_provider @@ -112,7 +112,7 @@ def setup_log_provider() -> Optional[LoggerProvider]: candidate.add_log_record_processor(BatchLogRecordProcessor(exporter)) set_logger_provider(candidate) - provider = get_logger_provider() + provider = cast(LoggerProvider, get_logger_provider()) if provider is not candidate: logger.warning( From 990090fe2e369ef179cda65f5fa7b50a23c9ffe8 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 12 Aug 2026 10:13:18 -0700 Subject: [PATCH 10/15] chore: bump version to 0.44.0 --- pyproject.toml | 2 +- uv.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9baab7b9..55e8e297 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sap-cloud-sdk" -version = "0.43.2" +version = "0.44.0" description = "SAP Cloud SDK for Python" readme = "README.md" license = "Apache-2.0" diff --git a/uv.lock b/uv.lock index 643d657f..f09b53da 100644 --- a/uv.lock +++ b/uv.lock @@ -3925,7 +3925,7 @@ wheels = [ [[package]] name = "sap-cloud-sdk" -version = "0.43.2" +version = "0.44.0" source = { editable = "." } dependencies = [ { name = "cryptography" }, From 37ef724430c5be4e5e7fe8f70202259413016ea0 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 12 Aug 2026 12:50:54 -0700 Subject: [PATCH 11/15] fix(telemetry): inject sap.cloud_sdk.* resource attrs when platform owns LoggerProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the platform's opentelemetry-instrument wrapper pre-installs a LoggerProvider, our candidate is rejected by get_logger_provider() and the platform's resource is used for all log records — missing sap.cloud_sdk.language, sap.cloud_sdk.name, and sap.cloud_sdk.version. Add _SdkResourceEnrichingProcessor, a LogRecordProcessor wrapper that merges the SDK resource into every ReadWriteLogRecord at on_emit time before forwarding to the inner BatchLogRecordProcessor. Use it in the fallback path of setup_log_provider() when provider is not candidate, so SDK resource attributes always appear in exported records regardless of which LoggerProvider is the global one. --- src/sap_cloud_sdk/core/telemetry/_provider.py | 43 ++++++++- tests/core/unit/telemetry/test_provider.py | 89 +++++++++++++++++-- 2 files changed, 121 insertions(+), 11 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 9ee149c8..adf2f65e 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -19,7 +19,11 @@ OTLPMetricExporter as HTTPMetricExporter, ) from opentelemetry.instrumentation.logging.handler import LoggingHandler -from opentelemetry.sdk._logs import LoggerProvider +from opentelemetry.sdk._logs import ( + LoggerProvider, + LogRecordProcessor, + ReadWriteLogRecord, +) from opentelemetry.sdk._logs.export import BatchLogRecordProcessor from opentelemetry.sdk.metrics import ( MeterProvider, @@ -46,6 +50,35 @@ logger = logging.getLogger(__name__) + +class _SdkResourceEnrichingProcessor(LogRecordProcessor): + """Merges sap.cloud_sdk.* resource attributes into every log record at emit time. + + Used when the platform's auto-instrumentation has already installed a + LoggerProvider whose resource lacks SAP SDK attributes. Wraps a + BatchLogRecordProcessor and merges the SDK resource into each record + before forwarding, so sap.cloud_sdk.language/name/version always appear + in exported records regardless of which LoggerProvider is the global one. + """ + + def __init__(self, inner: LogRecordProcessor, sdk_resource: "Resource") -> None: + self._inner = inner + self._sdk_resource = sdk_resource + + def on_emit(self, log_record: ReadWriteLogRecord) -> None: + if log_record.resource is not None: + log_record.resource = log_record.resource.merge(self._sdk_resource) + else: + log_record.resource = self._sdk_resource + self._inner.on_emit(log_record) + + def shutdown(self) -> None: + self._inner.shutdown() + + def force_flush(self, timeout_millis: int = 30000) -> bool: + return self._inner.force_flush(timeout_millis) + + # Global meter provider _meter_provider: Optional[MeterProvider] = None _meter: Optional[metrics.Meter] = None @@ -117,9 +150,13 @@ def setup_log_provider() -> Optional[LoggerProvider]: if provider is not candidate: logger.warning( "Global LoggerProvider was already set by another library. " - "Attaching SAP log processor to the existing provider." + "Attaching SAP log processor with resource enrichment to the existing provider." + ) + provider.add_log_record_processor( + _SdkResourceEnrichingProcessor( + BatchLogRecordProcessor(exporter), resource + ) ) - provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) handler = LoggingHandler(logger_provider=provider) logging.getLogger().addHandler(handler) diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index cddbeb3f..953e01bb 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -14,6 +14,9 @@ ) from opentelemetry.sdk.metrics.export import AggregationTemporality +from opentelemetry.sdk._logs import ReadWriteLogRecord +from opentelemetry.sdk._logs._internal import LogRecord + from sap_cloud_sdk.core.telemetry._provider import ( get_meter, shutdown, @@ -21,6 +24,7 @@ _create_metric_exporter, setup_log_provider, _create_log_exporter, + _SdkResourceEnrichingProcessor, ) from sap_cloud_sdk.core.telemetry.config import InstrumentationConfig @@ -306,7 +310,7 @@ def test_exception_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", side_effect=Exception("boom")): assert setup_log_provider() is None - def test_external_provider_gets_our_processor_attached(self): + def test_external_provider_gets_enriching_processor_attached(self): external_provider = MagicMock() candidate = MagicMock() with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): @@ -316,15 +320,84 @@ def test_external_provider_gets_our_processor_attached(self): with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=candidate): with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=external_provider): - with patch(_LOGGING_HANDLER) as mock_handler_cls: + with patch(_LOGGING_HANDLER): with patch("logging.getLogger"): result = setup_log_provider() - # returns the external provider, not our candidate assert result is external_provider - # BatchLogRecordProcessor called twice: once for our candidate, - # once to attach our processor to the external provider + # BatchLogRecordProcessor constructed twice: once for our candidate, + # once wrapped inside the enriching processor assert mock_proc.call_count == 2 - external_provider.add_log_record_processor.assert_called_once() - # handler wired to external provider - mock_handler_cls.assert_called_once_with(logger_provider=external_provider) + # The enriching processor (not a bare BatchLogRecordProcessor) is + # added to the external provider so SDK resource attrs are injected + call_args = external_provider.add_log_record_processor.call_args + assert call_args is not None + attached = call_args[0][0] + assert isinstance(attached, _SdkResourceEnrichingProcessor) + + +class TestSdkResourceEnrichingProcessor: + def _make_log_record(self, resource_attrs: dict): + from opentelemetry.sdk.resources import Resource as _Resource + from opentelemetry.sdk._logs._internal import LogRecord, ReadWriteLogRecord as _RWR + from opentelemetry._logs import SeverityNumber + log_record = LogRecord(severity_number=SeverityNumber.INFO, body="test") + return _RWR(log_record=log_record, resource=_Resource(resource_attrs)) + + def test_on_emit_merges_sdk_attrs(self): + from opentelemetry.sdk.resources import Resource as _Resource + + sdk_resource = _Resource({"sap.cloud_sdk.language": "python", "sap.cloud_sdk.name": "sap-cloud-sdk"}) + inner = MagicMock() + proc = _SdkResourceEnrichingProcessor(inner, sdk_resource) + + rw = self._make_log_record({"service.name": "my-service"}) + proc.on_emit(rw) + + inner.on_emit.assert_called_once_with(rw) + assert rw.resource.attributes["sap.cloud_sdk.language"] == "python" + assert rw.resource.attributes["sap.cloud_sdk.name"] == "sap-cloud-sdk" + # original attrs preserved + assert rw.resource.attributes["service.name"] == "my-service" + + def test_on_emit_sdk_attrs_win_on_collision(self): + from opentelemetry.sdk.resources import Resource as _Resource + + sdk_resource = _Resource({"service.name": "sdk-override", "sap.cloud_sdk.language": "python"}) + inner = MagicMock() + proc = _SdkResourceEnrichingProcessor(inner, sdk_resource) + + rw = self._make_log_record({"service.name": "platform-name"}) + proc.on_emit(rw) + + # SDK wins on collision + assert rw.resource.attributes["service.name"] == "sdk-override" + + def test_on_emit_null_resource_replaced(self): + from opentelemetry.sdk.resources import Resource as _Resource + from opentelemetry.sdk._logs._internal import LogRecord, ReadWriteLogRecord as _RWR + from opentelemetry._logs import SeverityNumber + + sdk_resource = _Resource({"sap.cloud_sdk.language": "python"}) + inner = MagicMock() + proc = _SdkResourceEnrichingProcessor(inner, sdk_resource) + + log_record = LogRecord(severity_number=SeverityNumber.INFO, body="test") + rw = _RWR(log_record=log_record, resource=None) + proc.on_emit(rw) + + assert rw.resource is sdk_resource + + def test_shutdown_delegates(self): + inner = MagicMock() + proc = _SdkResourceEnrichingProcessor(inner, MagicMock()) + proc.shutdown() + inner.shutdown.assert_called_once() + + def test_force_flush_delegates(self): + inner = MagicMock() + inner.force_flush.return_value = True + proc = _SdkResourceEnrichingProcessor(inner, MagicMock()) + result = proc.force_flush(5000) + inner.force_flush.assert_called_once_with(5000) + assert result is True From e25797239d5fe1a47214faa35a79ccf4953aca88 Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Wed, 12 Aug 2026 14:07:59 -0700 Subject: [PATCH 12/15] fix(telemetry): eliminate log duplication when platform owns the LoggerProvider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit opentelemetry-instrument (auto-instrumentation wrapper) calls set_logger_provider() before the application starts. set_logger_provider is a set-once API, so our subsequent call is silently ignored and the platform's provider is used instead. The previous _SdkResourceEnrichingProcessor approach attached a second processor to the provider and a second LoggingHandler to the root logger. Because the platform had already installed both, each log event produced four exported records (2 handlers × 2 processors), with two carrying SDK attrs and two carrying only platform attrs. New approach: - Detect the platform path via isinstance(get_logger_provider(), LoggerProvider). ProxyLoggerProvider (returned before any provider is set) fails this check; the platform's concrete LoggerProvider passes it. - When the platform's provider is already set, mutate its _resource in-place and update all active Logger instances under _active_loggers_lock, exactly as auto_instrument.py does for TracerProvider. This ensures every record emitted by the platform's pipeline carries sap.cloud_sdk.* resource attributes. - Do not add a second processor or handler when one is already present. - Only add processor + handler when no LoggingHandler exists on the root logger (covers the case where the platform set a provider but skipped the handler). Result: exactly one exported record per log event, with all SDK resource attributes. --- src/sap_cloud_sdk/core/telemetry/_provider.py | 79 +++--- tests/core/unit/telemetry/test_provider.py | 251 +++++++++--------- 2 files changed, 155 insertions(+), 175 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index adf2f65e..31b6d4dd 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -19,11 +19,7 @@ OTLPMetricExporter as HTTPMetricExporter, ) from opentelemetry.instrumentation.logging.handler import LoggingHandler -from opentelemetry.sdk._logs import ( - LoggerProvider, - LogRecordProcessor, - ReadWriteLogRecord, -) +from opentelemetry.sdk._logs import LoggerProvider from opentelemetry.sdk._logs.export import BatchLogRecordProcessor from opentelemetry.sdk.metrics import ( MeterProvider, @@ -51,32 +47,26 @@ logger = logging.getLogger(__name__) -class _SdkResourceEnrichingProcessor(LogRecordProcessor): - """Merges sap.cloud_sdk.* resource attributes into every log record at emit time. +def _merge_sdk_resource_into_log_provider( + provider: LoggerProvider, sdk_resource: Resource +) -> None: + """Mutate provider._resource and update all active Logger instances. - Used when the platform's auto-instrumentation has already installed a - LoggerProvider whose resource lacks SAP SDK attributes. Wraps a - BatchLogRecordProcessor and merges the SDK resource into each record - before forwarding, so sap.cloud_sdk.language/name/version always appear - in exported records regardless of which LoggerProvider is the global one. + Mirrors the TracerProvider resource merge in auto_instrument.py — + OTel SDK exposes no public API to swap a LoggerProvider's Resource + post-construction. """ + provider._resource = provider.resource.merge(sdk_resource) + with provider._active_loggers_lock: + for logger_instance in provider._active_loggers: + logger_instance._resource = provider._resource + logger.info( + "Merged sap-cloud-sdk resource attrs onto wrapper-installed LoggerProvider" + ) - def __init__(self, inner: LogRecordProcessor, sdk_resource: "Resource") -> None: - self._inner = inner - self._sdk_resource = sdk_resource - - def on_emit(self, log_record: ReadWriteLogRecord) -> None: - if log_record.resource is not None: - log_record.resource = log_record.resource.merge(self._sdk_resource) - else: - log_record.resource = self._sdk_resource - self._inner.on_emit(log_record) - def shutdown(self) -> None: - self._inner.shutdown() - - def force_flush(self, timeout_millis: int = 30000) -> bool: - return self._inner.force_flush(timeout_millis) +def _root_logger_has_otel_handler() -> bool: + return any(isinstance(h, LoggingHandler) for h in logging.getLogger().handlers) # Global meter provider @@ -141,33 +131,36 @@ def setup_log_provider() -> Optional[LoggerProvider]: try: resource = Resource.create(create_resource_attributes_from_env()) exporter = _create_log_exporter() - candidate = LoggerProvider(resource=resource) - candidate.add_log_record_processor(BatchLogRecordProcessor(exporter)) - set_logger_provider(candidate) - provider = cast(LoggerProvider, get_logger_provider()) + existing = cast(LoggerProvider, get_logger_provider()) - if provider is not candidate: + if isinstance(existing, LoggerProvider): + # Platform's auto-instrumentation pre-installed a provider. + # Merge SDK resource attrs into it so all records carry sap.cloud_sdk.*. + # Do not add a second processor or handler — the platform already installed + # both, and adding duplicates causes multiple exports per log event. logger.warning( "Global LoggerProvider was already set by another library. " - "Attaching SAP log processor with resource enrichment to the existing provider." - ) - provider.add_log_record_processor( - _SdkResourceEnrichingProcessor( - BatchLogRecordProcessor(exporter), resource - ) + "Merging sap.cloud_sdk.* resource attributes into the existing provider." ) + _merge_sdk_resource_into_log_provider(existing, resource) + if not _root_logger_has_otel_handler(): + existing.add_log_record_processor(BatchLogRecordProcessor(exporter)) + logging.getLogger().addHandler(LoggingHandler(logger_provider=existing)) + _log_provider = existing + else: + provider = LoggerProvider(resource=resource) + provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) + set_logger_provider(provider) + logging.getLogger().addHandler(LoggingHandler(logger_provider=provider)) + _log_provider = provider - handler = LoggingHandler(logger_provider=provider) - logging.getLogger().addHandler(handler) - - _log_provider = provider logger.info( f"OpenTelemetry log provider initialized. " f"Service: {config.service_name}, " f"Endpoint: {config.otlp_endpoint}" ) - return provider + return _log_provider except Exception as e: logger.error(f"Failed to initialize OpenTelemetry log provider: {e}") diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index 953e01bb..1d138013 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -14,8 +14,7 @@ ) from opentelemetry.sdk.metrics.export import AggregationTemporality -from opentelemetry.sdk._logs import ReadWriteLogRecord -from opentelemetry.sdk._logs._internal import LogRecord +from opentelemetry.sdk.resources import Resource from sap_cloud_sdk.core.telemetry._provider import ( get_meter, @@ -24,7 +23,8 @@ _create_metric_exporter, setup_log_provider, _create_log_exporter, - _SdkResourceEnrichingProcessor, + _merge_sdk_resource_into_log_provider, + _root_logger_has_otel_handler, ) from sap_cloud_sdk.core.telemetry.config import InstrumentationConfig @@ -246,63 +246,49 @@ def test_disabled_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=config): assert setup_log_provider() is None - def test_returns_configured_provider(self): - mock_provider = MagicMock() + def test_normal_path_sets_our_provider(self): + """No pre-installed provider — we create ours, set it globally, add handler.""" 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_log_exporter"): with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): - with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=mock_provider): - with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): - with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=mock_provider): - with patch(_LOGGING_HANDLER): - with patch("logging.getLogger"): - result = setup_log_provider() - assert result is mock_provider + # plain MagicMock fails isinstance(x, LoggerProvider) → normal path + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=MagicMock()): + with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider") as mock_set: + with patch(_LOGGING_HANDLER): + with patch("logging.getLogger"): + result = setup_log_provider() + assert result is not None + mock_set.assert_called_once_with(result) - def test_uses_shared_resource_attributes(self): + def test_normal_path_uses_sdk_resource(self): with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): with patch("sap_cloud_sdk.core.telemetry._provider.create_resource_attributes_from_env", return_value={"service.name": "svc"}) as mock_attrs: with patch("sap_cloud_sdk.core.telemetry._provider.Resource") as mock_resource: with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter"): with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): - with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider"): - with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): - with patch(_LOGGING_HANDLER): - with patch("logging.getLogger"): - setup_log_provider() - mock_attrs.assert_called_once() - mock_resource.create.assert_called_once_with({"service.name": "svc"}) - - def test_calls_set_logger_provider(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("sap_cloud_sdk.core.telemetry._provider._create_log_exporter"): - with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): - mock_provider = MagicMock() - with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=mock_provider): - with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider") as mock_set: - with patch(_LOGGING_HANDLER): - with patch("logging.getLogger"): - setup_log_provider() - mock_set.assert_called_once_with(mock_provider) - - def test_installs_handler_on_root_logger(self): + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=MagicMock()): + with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider"): + with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): + with patch(_LOGGING_HANDLER): + with patch("logging.getLogger"): + setup_log_provider() + mock_attrs.assert_called_once() + mock_resource.create.assert_called_once_with({"service.name": "svc"}) + + def test_normal_path_installs_handler_on_root_logger(self): + mock_handler = MagicMock() + mock_root = 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_log_exporter"): with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): - mock_provider = MagicMock() - with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=mock_provider): + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=MagicMock()): with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): - with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=mock_provider): - mock_handler = MagicMock() - with patch(_LOGGING_HANDLER, return_value=mock_handler): - mock_root = MagicMock() - with patch("logging.getLogger", return_value=mock_root) as mock_get_logger: - setup_log_provider() - mock_get_logger.assert_called_once_with() - mock_root.addHandler.assert_called_once_with(mock_handler) + with patch(_LOGGING_HANDLER, return_value=mock_handler): + with patch("logging.getLogger", return_value=mock_root): + setup_log_provider() + mock_root.addHandler.assert_called_once_with(mock_handler) def test_exception_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider.get_config", return_value=_ENABLED_CONFIG): @@ -310,94 +296,95 @@ def test_exception_returns_none(self): with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", side_effect=Exception("boom")): assert setup_log_provider() is None - def test_external_provider_gets_enriching_processor_attached(self): - external_provider = MagicMock() - candidate = MagicMock() + def test_platform_path_merges_resource_no_extra_handler(self): + """Platform pre-installed provider with a handler — merge resource, add nothing.""" + from opentelemetry.sdk._logs import LoggerProvider as _LP + external = MagicMock(spec=_LP) 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_log_exporter"): - with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor") as mock_proc: - with patch("sap_cloud_sdk.core.telemetry._provider.LoggerProvider", return_value=candidate): - with patch("sap_cloud_sdk.core.telemetry._provider.set_logger_provider"): - with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=external_provider): - with patch(_LOGGING_HANDLER): + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=external): + with patch("sap_cloud_sdk.core.telemetry._provider._merge_sdk_resource_into_log_provider") as mock_merge: + with patch("sap_cloud_sdk.core.telemetry._provider._root_logger_has_otel_handler", return_value=True): + result = setup_log_provider() + assert result is external + mock_merge.assert_called_once() + external.add_log_record_processor.assert_not_called() + + def test_platform_path_adds_handler_when_none_present(self): + """Platform set provider but no LoggingHandler — we add our own.""" + from opentelemetry.sdk._logs import LoggerProvider as _LP + external = MagicMock(spec=_LP) + 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_log_exporter"): + with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=external): + with patch("sap_cloud_sdk.core.telemetry._provider._merge_sdk_resource_into_log_provider"): + with patch("sap_cloud_sdk.core.telemetry._provider._root_logger_has_otel_handler", return_value=False): + with patch(_LOGGING_HANDLER) as mock_handler_cls: with patch("logging.getLogger"): - result = setup_log_provider() - - assert result is external_provider - # BatchLogRecordProcessor constructed twice: once for our candidate, - # once wrapped inside the enriching processor - assert mock_proc.call_count == 2 - # The enriching processor (not a bare BatchLogRecordProcessor) is - # added to the external provider so SDK resource attrs are injected - call_args = external_provider.add_log_record_processor.call_args - assert call_args is not None - attached = call_args[0][0] - assert isinstance(attached, _SdkResourceEnrichingProcessor) - - -class TestSdkResourceEnrichingProcessor: - def _make_log_record(self, resource_attrs: dict): - from opentelemetry.sdk.resources import Resource as _Resource - from opentelemetry.sdk._logs._internal import LogRecord, ReadWriteLogRecord as _RWR - from opentelemetry._logs import SeverityNumber - log_record = LogRecord(severity_number=SeverityNumber.INFO, body="test") - return _RWR(log_record=log_record, resource=_Resource(resource_attrs)) - - def test_on_emit_merges_sdk_attrs(self): - from opentelemetry.sdk.resources import Resource as _Resource - - sdk_resource = _Resource({"sap.cloud_sdk.language": "python", "sap.cloud_sdk.name": "sap-cloud-sdk"}) - inner = MagicMock() - proc = _SdkResourceEnrichingProcessor(inner, sdk_resource) - - rw = self._make_log_record({"service.name": "my-service"}) - proc.on_emit(rw) - - inner.on_emit.assert_called_once_with(rw) - assert rw.resource.attributes["sap.cloud_sdk.language"] == "python" - assert rw.resource.attributes["sap.cloud_sdk.name"] == "sap-cloud-sdk" - # original attrs preserved - assert rw.resource.attributes["service.name"] == "my-service" - - def test_on_emit_sdk_attrs_win_on_collision(self): - from opentelemetry.sdk.resources import Resource as _Resource - - sdk_resource = _Resource({"service.name": "sdk-override", "sap.cloud_sdk.language": "python"}) - inner = MagicMock() - proc = _SdkResourceEnrichingProcessor(inner, sdk_resource) - - rw = self._make_log_record({"service.name": "platform-name"}) - proc.on_emit(rw) - - # SDK wins on collision - assert rw.resource.attributes["service.name"] == "sdk-override" - - def test_on_emit_null_resource_replaced(self): - from opentelemetry.sdk.resources import Resource as _Resource - from opentelemetry.sdk._logs._internal import LogRecord, ReadWriteLogRecord as _RWR - from opentelemetry._logs import SeverityNumber - - sdk_resource = _Resource({"sap.cloud_sdk.language": "python"}) - inner = MagicMock() - proc = _SdkResourceEnrichingProcessor(inner, sdk_resource) - - log_record = LogRecord(severity_number=SeverityNumber.INFO, body="test") - rw = _RWR(log_record=log_record, resource=None) - proc.on_emit(rw) - - assert rw.resource is sdk_resource - - def test_shutdown_delegates(self): - inner = MagicMock() - proc = _SdkResourceEnrichingProcessor(inner, MagicMock()) - proc.shutdown() - inner.shutdown.assert_called_once() - - def test_force_flush_delegates(self): - inner = MagicMock() - inner.force_flush.return_value = True - proc = _SdkResourceEnrichingProcessor(inner, MagicMock()) - result = proc.force_flush(5000) - inner.force_flush.assert_called_once_with(5000) - assert result is True + setup_log_provider() + external.add_log_record_processor.assert_called_once() + mock_handler_cls.assert_called_once_with(logger_provider=external) + + +class TestMergeSdkResourceIntoLogProvider: + def test_updates_provider_resource(self): + from opentelemetry.sdk._logs import LoggerProvider as _LP + from opentelemetry.sdk.resources import Resource as _R + + sdk_resource = _R({"sap.cloud_sdk.language": "python"}) + provider = _LP(resource=_R({"service.name": "svc"})) + + _merge_sdk_resource_into_log_provider(provider, sdk_resource) + + assert provider._resource.attributes["sap.cloud_sdk.language"] == "python" + assert provider._resource.attributes["service.name"] == "svc" + + def test_updates_active_logger_resources(self): + from opentelemetry.sdk._logs import LoggerProvider as _LP + from opentelemetry.sdk.resources import Resource as _R + + sdk_resource = _R({"sap.cloud_sdk.language": "python"}) + provider = _LP(resource=_R({"service.name": "svc"})) + logger_instance = provider.get_logger("test.module") + + _merge_sdk_resource_into_log_provider(provider, sdk_resource) + + # The logger already in the active set gets the updated resource + assert logger_instance._resource.attributes["sap.cloud_sdk.language"] == "python" # ty: ignore[unresolved-attribute] + + def test_sdk_attrs_win_on_collision(self): + from opentelemetry.sdk._logs import LoggerProvider as _LP + from opentelemetry.sdk.resources import Resource as _R + + sdk_resource = _R({"service.name": "sdk-name"}) + provider = _LP(resource=_R({"service.name": "platform-name"})) + + _merge_sdk_resource_into_log_provider(provider, sdk_resource) + + assert provider._resource.attributes["service.name"] == "sdk-name" + + +class TestRootLoggerHasOtelHandler: + def test_returns_false_when_no_handler(self): + root = logging.getLogger() + original = root.handlers[:] + root.handlers = [] + try: + assert _root_logger_has_otel_handler() is False + finally: + root.handlers = original + + def test_returns_true_when_handler_present(self): + from opentelemetry.instrumentation.logging.handler import LoggingHandler as OtelHandler + root = logging.getLogger() + original = root.handlers[:] + mock_provider = MagicMock() + handler = OtelHandler(logger_provider=mock_provider) + root.handlers = [handler] + try: + assert _root_logger_has_otel_handler() is True + finally: + root.handlers = original From 9853613c82bb5917ea7fa848564875b5f02e0f2f Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Thu, 13 Aug 2026 10:31:35 -0700 Subject: [PATCH 13/15] fix(telemetry): eliminate 4x log duplication with platform auto-instrumentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When OTEL_PYTHON_LOGGING_AUTO_INSTRUMENTATION_ENABLED=true, the platform's sitecustomize.py installs opentelemetry.sdk._logs.LoggingHandler, which is a different class from opentelemetry.instrumentation.logging.handler.LoggingHandler. Our isinstance check in _root_logger_has_otel_handler() missed it, causing us to add a second BatchLogRecordProcessor AND a second LoggingHandler on top of the platform's. Combined with LoggingInstrumentor later adding yet another handler via _instrument_libraries(), this produced 4 records per log event. Fixes: 1. _root_logger_has_otel_handler() now checks both handler classes. 2. Platform path no longer adds a BatchLogRecordProcessor — the platform LP already has one; adding a second one doubles every exported record. 3. LoggingInstrumentorWrapper._instrument() passes enable_log_auto_instrumentation=False when an OTel handler already exists, allowing trace-context injection without adding a duplicate handler. --- src/sap_cloud_sdk/core/telemetry/_provider.py | 24 +++++++++---- .../instrumentation/instrumentors/logging.py | 27 +++++++++++++- tests/core/unit/telemetry/test_provider.py | 35 +++++++++++++------ 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 31b6d4dd..2ffe3901 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -66,7 +66,16 @@ def _merge_sdk_resource_into_log_provider( def _root_logger_has_otel_handler() -> bool: - return any(isinstance(h, LoggingHandler) for h in logging.getLogger().handlers) + # The platform's sitecustomize.py (OTEL_PYTHON_LOGGING_AUTO_INSTRUMENTATION_ENABLED) + # installs opentelemetry.sdk._logs.LoggingHandler, which is a different class from + # opentelemetry.instrumentation.logging.handler.LoggingHandler. Check both. + try: + from opentelemetry.sdk._logs import LoggingHandler as _SDKLoggingHandler + + handler_types: tuple[type, ...] = (LoggingHandler, _SDKLoggingHandler) + except ImportError: + handler_types = (LoggingHandler,) + return any(isinstance(h, handler_types) for h in logging.getLogger().handlers) # Global meter provider @@ -130,27 +139,28 @@ def setup_log_provider() -> Optional[LoggerProvider]: try: resource = Resource.create(create_resource_attributes_from_env()) - exporter = _create_log_exporter() - existing = cast(LoggerProvider, get_logger_provider()) if isinstance(existing, LoggerProvider): # Platform's auto-instrumentation pre-installed a provider. # Merge SDK resource attrs into it so all records carry sap.cloud_sdk.*. - # Do not add a second processor or handler — the platform already installed - # both, and adding duplicates causes multiple exports per log event. + # Never add a second BatchLogRecordProcessor here — the platform's provider + # already has one, and a second processor doubles every exported log record. logger.warning( "Global LoggerProvider was already set by another library. " "Merging sap.cloud_sdk.* resource attributes into the existing provider." ) _merge_sdk_resource_into_log_provider(existing, resource) if not _root_logger_has_otel_handler(): - existing.add_log_record_processor(BatchLogRecordProcessor(exporter)) + # No stdlib bridge handler yet — add one so log records reach the + # platform's existing processor. No extra processor needed. logging.getLogger().addHandler(LoggingHandler(logger_provider=existing)) _log_provider = existing else: provider = LoggerProvider(resource=resource) - provider.add_log_record_processor(BatchLogRecordProcessor(exporter)) + provider.add_log_record_processor( + BatchLogRecordProcessor(_create_log_exporter()) + ) set_logger_provider(provider) logging.getLogger().addHandler(LoggingHandler(logger_provider=provider)) _log_provider = provider diff --git a/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py b/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py index 72c93995..7de268c1 100644 --- a/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py +++ b/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py @@ -1,4 +1,9 @@ +import logging as stdlib_logging + from opentelemetry.instrumentation.logging import LoggingInstrumentor +from opentelemetry.instrumentation.logging.handler import ( + LoggingHandler as _InstrumentationHandler, +) from sap_cloud_sdk.core.telemetry.instrumentation.base import LibraryInstrumentor from sap_cloud_sdk.core.telemetry.instrumentation._registry import register @@ -6,6 +11,19 @@ _instrumentor = LoggingInstrumentor() +def _has_otel_handler_on_root() -> bool: + """Return True if any OTel log bridge handler is already on the root logger.""" + try: + from opentelemetry.sdk._logs import LoggingHandler as _SDKHandler + + handler_types: tuple[type, ...] = (_InstrumentationHandler, _SDKHandler) + except ImportError: + handler_types = (_InstrumentationHandler,) + return any( + isinstance(h, handler_types) for h in stdlib_logging.getLogger().handlers + ) + + class LoggingInstrumentorWrapper(LibraryInstrumentor): """Injects trace_id and span_id into every stdlib log record for log-trace correlation.""" @@ -15,7 +33,14 @@ def is_instrumented(self) -> bool: return _instrumentor.is_instrumented_by_opentelemetry def _instrument(self, **kwargs) -> None: - _instrumentor.instrument(set_logging_format=True) + if _has_otel_handler_on_root(): + # An OTel log bridge handler is already on root (from platform + # auto-instrumentation or setup_log_provider). Pass + # enable_log_auto_instrumentation=False so LoggingInstrumentor + # only injects trace context into stdlib log records — it must + # not add a second handler that would duplicate every log record. + kwargs = {**kwargs, "enable_log_auto_instrumentation": False} + _instrumentor.instrument(**kwargs) def _uninstrument(self) -> None: _instrumentor.uninstrument() diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index 1d138013..8c7437bf 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -312,21 +312,20 @@ def test_platform_path_merges_resource_no_extra_handler(self): external.add_log_record_processor.assert_not_called() def test_platform_path_adds_handler_when_none_present(self): - """Platform set provider but no LoggingHandler — we add our own.""" + """Platform set provider but no LoggingHandler — we add handler, no extra processor.""" from opentelemetry.sdk._logs import LoggerProvider as _LP external = MagicMock(spec=_LP) 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_log_exporter"): - with patch("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor"): - with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=external): - with patch("sap_cloud_sdk.core.telemetry._provider._merge_sdk_resource_into_log_provider"): - with patch("sap_cloud_sdk.core.telemetry._provider._root_logger_has_otel_handler", return_value=False): - with patch(_LOGGING_HANDLER) as mock_handler_cls: - with patch("logging.getLogger"): - setup_log_provider() - external.add_log_record_processor.assert_called_once() - mock_handler_cls.assert_called_once_with(logger_provider=external) + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=external): + with patch("sap_cloud_sdk.core.telemetry._provider._merge_sdk_resource_into_log_provider"): + with patch("sap_cloud_sdk.core.telemetry._provider._root_logger_has_otel_handler", return_value=False): + with patch(_LOGGING_HANDLER) as mock_handler_cls: + with patch("logging.getLogger"): + setup_log_provider() + # No second processor — platform LP already has one + external.add_log_record_processor.assert_not_called() + mock_handler_cls.assert_called_once_with(logger_provider=external) class TestMergeSdkResourceIntoLogProvider: @@ -388,3 +387,17 @@ def test_returns_true_when_handler_present(self): assert _root_logger_has_otel_handler() is True finally: root.handlers = original + + def test_returns_true_when_sdk_level_handler_present(self): + """Platform sitecustomize.py uses opentelemetry.sdk._logs.LoggingHandler, not the + instrumentation-layer one. _root_logger_has_otel_handler must detect both.""" + from opentelemetry.sdk._logs import LoggingHandler as SDKHandler + root = logging.getLogger() + original = root.handlers[:] + mock_provider = MagicMock() + handler = SDKHandler(logger_provider=mock_provider) + root.handlers = [handler] + try: + assert _root_logger_has_otel_handler() is True + finally: + root.handlers = original From db532be5aaa9f5a218a2bf6b5852ad7bab55f2ed Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Thu, 13 Aug 2026 13:12:33 -0700 Subject: [PATCH 14/15] fix(telemetry): fix CI failures after rebase onto main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Restore set_logging_format=True in LoggingInstrumentorWrapper - Update test_exception_returns_none to force the else path so the exception in _create_log_exporter is actually reached - Update clashing-provider test to assert against external_exporter — we no longer add a second processor to the platform LP - Update subaccount_id attribute key after rename on main - Clean up what-comments, keep only the non-obvious why --- src/sap_cloud_sdk/core/telemetry/_provider.py | 17 +++-------------- .../instrumentation/instrumentors/logging.py | 9 +++------ .../unit/telemetry/test_log_provider_e2e.py | 4 ++-- tests/core/unit/telemetry/test_provider.py | 8 +++----- 4 files changed, 11 insertions(+), 27 deletions(-) diff --git a/src/sap_cloud_sdk/core/telemetry/_provider.py b/src/sap_cloud_sdk/core/telemetry/_provider.py index 2ffe3901..05851ab1 100644 --- a/src/sap_cloud_sdk/core/telemetry/_provider.py +++ b/src/sap_cloud_sdk/core/telemetry/_provider.py @@ -50,12 +50,7 @@ def _merge_sdk_resource_into_log_provider( provider: LoggerProvider, sdk_resource: Resource ) -> None: - """Mutate provider._resource and update all active Logger instances. - - Mirrors the TracerProvider resource merge in auto_instrument.py — - OTel SDK exposes no public API to swap a LoggerProvider's Resource - post-construction. - """ + """OTel SDK has no public API to swap a LoggerProvider's Resource after construction.""" provider._resource = provider.resource.merge(sdk_resource) with provider._active_loggers_lock: for logger_instance in provider._active_loggers: @@ -66,9 +61,7 @@ def _merge_sdk_resource_into_log_provider( def _root_logger_has_otel_handler() -> bool: - # The platform's sitecustomize.py (OTEL_PYTHON_LOGGING_AUTO_INSTRUMENTATION_ENABLED) - # installs opentelemetry.sdk._logs.LoggingHandler, which is a different class from - # opentelemetry.instrumentation.logging.handler.LoggingHandler. Check both. + # sitecustomize.py installs sdk._logs.LoggingHandler, not the instrumentation-layer one — check both. try: from opentelemetry.sdk._logs import LoggingHandler as _SDKLoggingHandler @@ -142,9 +135,7 @@ def setup_log_provider() -> Optional[LoggerProvider]: existing = cast(LoggerProvider, get_logger_provider()) if isinstance(existing, LoggerProvider): - # Platform's auto-instrumentation pre-installed a provider. - # Merge SDK resource attrs into it so all records carry sap.cloud_sdk.*. - # Never add a second BatchLogRecordProcessor here — the platform's provider + # Never add a second BatchLogRecordProcessor — the platform's provider # already has one, and a second processor doubles every exported log record. logger.warning( "Global LoggerProvider was already set by another library. " @@ -152,8 +143,6 @@ def setup_log_provider() -> Optional[LoggerProvider]: ) _merge_sdk_resource_into_log_provider(existing, resource) if not _root_logger_has_otel_handler(): - # No stdlib bridge handler yet — add one so log records reach the - # platform's existing processor. No extra processor needed. logging.getLogger().addHandler(LoggingHandler(logger_provider=existing)) _log_provider = existing else: diff --git a/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py b/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py index 7de268c1..97d11827 100644 --- a/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py +++ b/src/sap_cloud_sdk/core/telemetry/instrumentation/instrumentors/logging.py @@ -12,7 +12,7 @@ def _has_otel_handler_on_root() -> bool: - """Return True if any OTel log bridge handler is already on the root logger.""" + # sitecustomize.py installs sdk._logs.LoggingHandler, not the instrumentation-layer one — check both. try: from opentelemetry.sdk._logs import LoggingHandler as _SDKHandler @@ -33,12 +33,9 @@ def is_instrumented(self) -> bool: return _instrumentor.is_instrumented_by_opentelemetry def _instrument(self, **kwargs) -> None: + kwargs.setdefault("set_logging_format", True) if _has_otel_handler_on_root(): - # An OTel log bridge handler is already on root (from platform - # auto-instrumentation or setup_log_provider). Pass - # enable_log_auto_instrumentation=False so LoggingInstrumentor - # only injects trace context into stdlib log records — it must - # not add a second handler that would duplicate every log record. + # Already have a handler — adding another duplicates every log record. kwargs = {**kwargs, "enable_log_auto_instrumentation": False} _instrumentor.instrument(**kwargs) diff --git a/tests/core/unit/telemetry/test_log_provider_e2e.py b/tests/core/unit/telemetry/test_log_provider_e2e.py index 0b14aa14..5d8baef0 100644 --- a/tests/core/unit/telemetry/test_log_provider_e2e.py +++ b/tests/core/unit/telemetry/test_log_provider_e2e.py @@ -118,7 +118,7 @@ def test_resource_attributes_on_record(self, log_exporter): assert attrs.get("service.name") == "test-svc" assert attrs.get("sap.cloud_sdk.language") == "python" assert attrs.get("cloud.region") == "eu10" - assert attrs.get("sap.cld.subaccount_id") == "sub-123" + assert attrs.get("sap.cloud.provider.subaccount_id") == "sub-123" def test_extra_fields_become_log_attributes(self, log_exporter): logging.getLogger("test.extra").warning( @@ -196,7 +196,7 @@ def test_logs_reach_our_exporter_when_provider_already_set(self, monkeypatch): root.setLevel(logging.DEBUG) logging.getLogger("test.clash").warning("hello from sdk") - our_records = our_exporter.get_finished_logs() + our_records = external_exporter.get_finished_logs() assert len(our_records) == 1 assert our_records[0].log_record.body == "hello from sdk" diff --git a/tests/core/unit/telemetry/test_provider.py b/tests/core/unit/telemetry/test_provider.py index 8c7437bf..7d0d49af 100644 --- a/tests/core/unit/telemetry/test_provider.py +++ b/tests/core/unit/telemetry/test_provider.py @@ -293,8 +293,9 @@ def test_normal_path_installs_handler_on_root_logger(self): def test_exception_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("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", side_effect=Exception("boom")): - assert setup_log_provider() is None + with patch("sap_cloud_sdk.core.telemetry._provider.get_logger_provider", return_value=MagicMock()): + with patch("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", side_effect=Exception("boom")): + assert setup_log_provider() is None def test_platform_path_merges_resource_no_extra_handler(self): """Platform pre-installed provider with a handler — merge resource, add nothing.""" @@ -323,7 +324,6 @@ def test_platform_path_adds_handler_when_none_present(self): with patch(_LOGGING_HANDLER) as mock_handler_cls: with patch("logging.getLogger"): setup_log_provider() - # No second processor — platform LP already has one external.add_log_record_processor.assert_not_called() mock_handler_cls.assert_called_once_with(logger_provider=external) @@ -389,8 +389,6 @@ def test_returns_true_when_handler_present(self): root.handlers = original def test_returns_true_when_sdk_level_handler_present(self): - """Platform sitecustomize.py uses opentelemetry.sdk._logs.LoggingHandler, not the - instrumentation-layer one. _root_logger_has_otel_handler must detect both.""" from opentelemetry.sdk._logs import LoggingHandler as SDKHandler root = logging.getLogger() original = root.handlers[:] From 660591bcb84640ea17967d041a17a67a88f22f0d Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Thu, 13 Aug 2026 13:16:11 -0700 Subject: [PATCH 15/15] =?UTF-8?q?test(telemetry):=20fix=20clashing-provide?= =?UTF-8?q?r=20test=20=E2=80=94=20remove=20dead=20setup,=20assert=20resour?= =?UTF-8?q?ce=20merge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../unit/telemetry/test_log_provider_e2e.py | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/tests/core/unit/telemetry/test_log_provider_e2e.py b/tests/core/unit/telemetry/test_log_provider_e2e.py index 5d8baef0..04a7ada4 100644 --- a/tests/core/unit/telemetry/test_log_provider_e2e.py +++ b/tests/core/unit/telemetry/test_log_provider_e2e.py @@ -148,36 +148,29 @@ def test_no_trace_correlation_outside_span(self, log_exporter): class TestLogProviderClashingProvider: - """When another library claims the global LoggerProvider first, our processor - must still be attached so logs reach the SAP OTLP endpoint.""" + """When the platform pre-claims the global LoggerProvider, we merge our resource + attributes into it and leave its processor chain intact.""" - def test_logs_reach_our_exporter_when_provider_already_set(self, monkeypatch): + def test_logs_flow_through_existing_provider_with_sdk_resource(self, monkeypatch): from opentelemetry._logs import _internal as _logs_internal from opentelemetry.sdk._logs.export import InMemoryLogRecordExporter, SimpleLogRecordProcessor import sap_cloud_sdk.core.telemetry._provider as provider_module - # Reset OTel singleton _logs_internal._LOGGER_PROVIDER_SET_ONCE._done = False _logs_internal._LOGGER_PROVIDER = None provider_module._log_provider = None - # Remove stale LoggingHandlers root = logging.getLogger() for h in list(root.handlers): if isinstance(h, LoggingHandler): root.removeHandler(h) - # Simulate another library claiming the provider first external_exporter = InMemoryLogRecordExporter() external_provider = LoggerProvider() external_provider.add_log_record_processor(SimpleLogRecordProcessor(external_exporter)) from opentelemetry._logs import set_logger_provider set_logger_provider(external_provider) - # Now our SDK runs setup_log_provider - our_exporter = InMemoryLogRecordExporter() - monkeypatch.setattr("sap_cloud_sdk.core.telemetry._provider._create_log_exporter", lambda: our_exporter) - monkeypatch.setattr("sap_cloud_sdk.core.telemetry._provider.BatchLogRecordProcessor", SimpleLogRecordProcessor) monkeypatch.setattr( "sap_cloud_sdk.core.telemetry._provider.get_config", lambda: __import__( @@ -196,11 +189,12 @@ def test_logs_reach_our_exporter_when_provider_already_set(self, monkeypatch): root.setLevel(logging.DEBUG) logging.getLogger("test.clash").warning("hello from sdk") - our_records = external_exporter.get_finished_logs() - assert len(our_records) == 1 - assert our_records[0].log_record.body == "hello from sdk" + records = external_exporter.get_finished_logs() + assert len(records) == 1 + assert records[0].log_record.body == "hello from sdk" + # Resource merge must have happened — sap.cloud_sdk.* attrs prove it + assert records[0].resource.attributes.get("sap.cloud_sdk.language") == "python" - # Cleanup for h in list(root.handlers): if isinstance(h, LoggingHandler): root.removeHandler(h)