fix(telemetry): fix CI failures after rebase onto main

- 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

Author: jeanscherf

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,18 +135,14 @@ 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. "
                 "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():
-                # 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:

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)
 

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"
 

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[:]