Detect SDK handler in install_log_bridge (#97)

chris-colinsky · web-flow · commit 03dd599ec0a3 · 2026-05-29T16:47:42.000-07:00
The install_log_bridge idempotency check only recognized the
instrumentation LoggingHandler (the class OA itself installs).
Applications that attached opentelemetry.sdk._logs.LoggingHandler
against the same LoggerProvider in their own logging setup got a
SECOND OTel-Logs handler from OA's bridge, doubling every record
through the OTLP export path. Console handlers are independent so
the duplication is OTLP-only, which made it slow to diagnose.

Widen the check to detect either OTel-Logs handler class wired to
the same provider. The correlation_id LogRecord factory still
installs regardless. Provider-scoped so applications running a
separate logs pipeline against a DIFFERENT LoggerProvider still
get OA's bridge attached against the OA provider.

Document the collision shape in docs/agent/non-obvious-shapes.md
and regenerate the bundled AGENTS.md.

Reported via downstream integration.
diff --git a/docs/agent/non-obvious-shapes.md b/docs/agent/non-obvious-shapes.md
@@ -100,6 +100,17 @@ if summary.timeout_reached:
 
 The compiled graph stays usable for subsequent invocations after a timed-out drain — workers are cancelled cleanly, no partial state leaks.
 
+### `install_log_bridge` skips its own handler when the application already attached one to the same `LoggerProvider`
+
+Two distinct classes both named `LoggingHandler` exist in the OTel Python ecosystem and both bridge stdlib log records to the OTel Logs SDK:
+
+- `opentelemetry.sdk._logs.LoggingHandler` (the SDK class). Typically attached by an application's own logging setup — e.g., a FastAPI `setup_logging(...)` step that wires up an OTLP-backed `LoggerProvider` for log export.
+- `opentelemetry.instrumentation.logging.handler.LoggingHandler` (the instrumentation class). What `openarmature.observability.otel.install_log_bridge` attaches when it runs.
+
+Different classes, same OTel-Logs export path. If both are attached against the same `LoggerProvider`, every stdlib log record fires through both handlers, both call `provider.get_logger(...).emit(...)`, and `BatchLogRecordProcessor` ships the record TWICE to the OTLP endpoint. The duplication is OTLP-only — a console handler attached separately is unaffected, which makes "OTLP rows are doubled, console isn't" a head-scratcher to diagnose.
+
+`install_log_bridge` detects either handler class against the same provider and skips its own `addHandler` accordingly; the `openarmature.correlation_id` LogRecord factory still installs. The check is provider-scoped, so an application that intentionally attaches a handler against a DIFFERENT `LoggerProvider` (a separate logs pipeline) still gets the OA bridge against the OA provider — the helper only dedups when the SAME provider would receive duplicate emissions.
+
 ### Three exception hierarchies; know which one your code catches
 
 `openarmature` exceptions split across three sibling hierarchies:
diff --git a/src/openarmature/AGENTS.md b/src/openarmature/AGENTS.md
@@ -963,6 +963,17 @@ if summary.timeout_reached:
 
 The compiled graph stays usable for subsequent invocations after a timed-out drain — workers are cancelled cleanly, no partial state leaks.
 
+### `install_log_bridge` skips its own handler when the application already attached one to the same `LoggerProvider`
+
+Two distinct classes both named `LoggingHandler` exist in the OTel Python ecosystem and both bridge stdlib log records to the OTel Logs SDK:
+
+- `opentelemetry.sdk._logs.LoggingHandler` (the SDK class). Typically attached by an application's own logging setup — e.g., a FastAPI `setup_logging(...)` step that wires up an OTLP-backed `LoggerProvider` for log export.
+- `opentelemetry.instrumentation.logging.handler.LoggingHandler` (the instrumentation class). What `openarmature.observability.otel.install_log_bridge` attaches when it runs.
+
+Different classes, same OTel-Logs export path. If both are attached against the same `LoggerProvider`, every stdlib log record fires through both handlers, both call `provider.get_logger(...).emit(...)`, and `BatchLogRecordProcessor` ships the record TWICE to the OTLP endpoint. The duplication is OTLP-only — a console handler attached separately is unaffected, which makes "OTLP rows are doubled, console isn't" a head-scratcher to diagnose.
+
+`install_log_bridge` detects either handler class against the same provider and skips its own `addHandler` accordingly; the `openarmature.correlation_id` LogRecord factory still installs. The check is provider-scoped, so an application that intentionally attaches a handler against a DIFFERENT `LoggerProvider` (a separate logs pipeline) still gets the OA bridge against the OA provider — the helper only dedups when the SAME provider would receive duplicate emissions.
+
 ### Three exception hierarchies; know which one your code catches
 
 `openarmature` exceptions split across three sibling hierarchies:
diff --git a/src/openarmature/observability/otel/logs.py b/src/openarmature/observability/otel/logs.py
@@ -95,24 +95,36 @@ def install_log_bridge(
     logger or handler dispatch, so every record gets the attribute
     regardless of which logger originated it.
 
-    Idempotent: re-calling is a no-op (we check for the existing
-    OA-tagged handler on the root logger AND for the OA-installed
-    factory marker on the current global factory).
+    Idempotent across both OTel-Logs handler classes. Two different
+    classes both named ``LoggingHandler`` exist in the OTel Python
+    ecosystem and both bridge stdlib records to the Logs SDK:
+
+    - :class:`opentelemetry.sdk._logs.LoggingHandler` (the SDK class,
+      what an application's own logging setup typically installs).
+    - :class:`opentelemetry.instrumentation.logging.handler.LoggingHandler`
+      (the instrumentation class, what this helper installs).
+
+    Different classes, same OTel-Logs export path. If an application
+    has already attached the SDK class against the same
+    :class:`LoggerProvider`, calling this helper would attach the
+    instrumentation class on top and every record would emit to OTLP
+    twice. The check below detects EITHER class against the same
+    provider and skips the ``addHandler`` step accordingly; the
+    correlation_id factory still installs. Re-calling with no prior
+    OA-installed handler is also a no-op via the OA marker check.
 
     The user retains responsibility for providing the
     :class:`LoggerProvider` (typically built with their preferred
     exporter; :class:`InMemoryLogRecordExporter` for tests,
     :class:`OTLPLogExporter` for production).
     """
-    from opentelemetry.instrumentation.logging.handler import LoggingHandler
+    from opentelemetry.instrumentation.logging.handler import (
+        LoggingHandler as _InstrLoggingHandler,
+    )
 
     root = logging.getLogger()
-    # Idempotency #1: don't double-add the OTel LoggingHandler.
-    handler_already_installed = any(
-        isinstance(h, LoggingHandler) and getattr(h, "_openarmature_installed", False) for h in root.handlers
-    )
-    if not handler_already_installed:
-        handler = LoggingHandler(level=level, logger_provider=provider)
+    if not _otel_logs_handler_already_bridges(root, provider):
+        handler = _InstrLoggingHandler(level=level, logger_provider=provider)
         # Direct assignment isn't typed on LoggingHandler; route
         # through ``object.__setattr__`` to avoid pyright's strict
         # attribute-access check without losing the idempotency-
@@ -123,6 +135,37 @@ def install_log_bridge(
     _install_correlation_id_factory()
 
 
+def _otel_logs_handler_already_bridges(root: logging.Logger, provider: LoggerProvider) -> bool:
+    """True iff the root logger already has an OTel-Logs
+    ``LoggingHandler`` (SDK class OR instrumentation class) wired to
+    ``provider`` — meaning every record will already reach the OTLP
+    export path and a second ``addHandler`` here would duplicate.
+
+    Handler-class isinstance covers the case where an application
+    attached the SDK handler in its own logging setup; the
+    ``_openarmature_installed`` marker covers the case where this
+    helper was already called previously. ``_logger_provider`` is
+    OTel-private on both handler classes today — if a future SDK
+    rename hides it, ``getattr`` returns ``None`` and we conclude
+    "doesn't bridge", falling back to adding our own handler. Worst
+    case is the pre-fix behavior (potential dup); we never crash.
+    """
+    from opentelemetry.instrumentation.logging.handler import (
+        LoggingHandler as _InstrLoggingHandler,
+    )
+    from opentelemetry.sdk._logs import LoggingHandler as _SDKLoggingHandler
+
+    handler_classes = (_SDKLoggingHandler, _InstrLoggingHandler)
+    for handler in root.handlers:
+        if not isinstance(handler, handler_classes):
+            continue
+        if getattr(handler, "_openarmature_installed", False):
+            return True
+        if getattr(handler, "_logger_provider", None) is provider:
+            return True
+    return False
+
+
 __all__ = [
     "install_log_bridge",
 ]
diff --git a/tests/unit/test_observability_otel.py b/tests/unit/test_observability_otel.py
@@ -695,6 +695,91 @@ def test_install_log_bridge_is_idempotent() -> None:
         logging.setLogRecordFactory(prior_factory)
 
 
+def test_install_log_bridge_skips_when_sdk_handler_already_attached() -> None:
+    """Downstream report (HyperDX integration): if an application's
+    own logging setup attached
+    :class:`opentelemetry.sdk._logs.LoggingHandler` against the same
+    :class:`LoggerProvider` BEFORE ``install_log_bridge`` runs, the
+    helper MUST NOT attach a second
+    :class:`opentelemetry.instrumentation.logging.handler.LoggingHandler`
+    against the same provider — both classes bridge to the same OTel
+    Logs SDK and a second attach causes every record to ship to OTLP
+    twice. The correlation_id factory still installs."""
+    from opentelemetry.sdk._logs import LoggerProvider
+    from opentelemetry.sdk._logs import LoggingHandler as _SDKLoggingHandler
+
+    root = logging.getLogger()
+    prior_handlers = list(root.handlers)
+    prior_factory = logging.getLogRecordFactory()
+    try:
+        provider = LoggerProvider()
+        # Simulate the application's setup: attach the SDK handler
+        # against `provider` BEFORE OA's bridge runs.
+        sdk_handler = _SDKLoggingHandler(level=logging.NOTSET, logger_provider=provider)
+        root.addHandler(sdk_handler)
+        handler_count_before = len(root.handlers)
+
+        install_log_bridge(provider)
+
+        # No new handler attached — the SDK handler already bridges
+        # to `provider`, so installing the instrumentation handler
+        # would duplicate every emission.
+        assert len(root.handlers) == handler_count_before, (
+            f"install_log_bridge MUST NOT add a second OTel-Logs handler when an "
+            f"SDK handler is already wired to the same provider; "
+            f"got {len(root.handlers)} handlers (was {handler_count_before})"
+        )
+        # The correlation_id factory MUST install regardless — that's
+        # what the helper is for once handler bridging is already
+        # taken care of by the application.
+        current_factory = logging.getLogRecordFactory()
+        assert getattr(current_factory, "_openarmature_correlation_factory", False), (
+            "correlation_id factory MUST install even when the OTel-Logs handler "
+            "is skipped (application already attached one)"
+        )
+    finally:
+        root.handlers[:] = prior_handlers
+        logging.setLogRecordFactory(prior_factory)
+
+
+def test_install_log_bridge_adds_handler_when_pre_attached_uses_different_provider() -> None:
+    """An application MAY intentionally attach an SDK handler against
+    a DIFFERENT :class:`LoggerProvider` (e.g., a console-only logs
+    setup separate from the OA-managed OTLP provider). The
+    idempotency check is scoped to the SAME provider, so OA's helper
+    DOES attach its own handler against the OA provider in that
+    case — no false-positive dedup that would silently break the OA
+    bridge."""
+    from opentelemetry.sdk._logs import LoggerProvider
+    from opentelemetry.sdk._logs import LoggingHandler as _SDKLoggingHandler
+
+    root = logging.getLogger()
+    prior_handlers = list(root.handlers)
+    prior_factory = logging.getLogRecordFactory()
+    try:
+        # Application's pre-attached SDK handler points at a DIFFERENT
+        # LoggerProvider — its own logs pipeline.
+        unrelated_provider = LoggerProvider()
+        unrelated_handler = _SDKLoggingHandler(level=logging.NOTSET, logger_provider=unrelated_provider)
+        root.addHandler(unrelated_handler)
+        handler_count_before = len(root.handlers)
+
+        # OA's bridge installs against its OWN provider.
+        oa_provider = LoggerProvider()
+        install_log_bridge(oa_provider)
+
+        # One new handler MUST appear — the OA-installed
+        # instrumentation handler against `oa_provider`. The
+        # pre-existing unrelated handler is unaffected.
+        assert len(root.handlers) == handler_count_before + 1, (
+            f"install_log_bridge MUST attach when no handler bridges to the "
+            f"target provider; got {len(root.handlers)} (was {handler_count_before})"
+        )
+    finally:
+        root.handlers[:] = prior_handlers
+        logging.setLogRecordFactory(prior_factory)
+
+
 def test_log_bridge_exports_records_with_correlation_id() -> None:
     """Spec §7 end-to-end: a log record emitted on a CHILD logger
     under ``current_correlation_id`` flows through the bridge to
