Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions docs/agent/non-obvious-shapes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
11 changes: 11 additions & 0 deletions src/openarmature/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
63 changes: 53 additions & 10 deletions src/openarmature/observability/otel/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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-
Expand All @@ -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",
]
85 changes: 85 additions & 0 deletions tests/unit/test_observability_otel.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down