Skip to content

feat(telemetry): add OTel standard logs as third signal - #239

Open
jeanscherf wants to merge 15 commits into
mainfrom
feat/otel-standard-logs
Open

feat(telemetry): add OTel standard logs as third signal#239
jeanscherf wants to merge 15 commits into
mainfrom
feat/otel-standard-logs

Conversation

@jeanscherf

@jeanscherf jeanscherf commented Jul 21, 2026

Copy link
Copy Markdown
Member

Summary

  • Wires up a LoggerProvider alongside existing metrics and traces using the shared resource attributes (service.name, region, subaccount, etc.)
  • auto_instrument() now sets up all three OTel signals — no app boilerplate changes needed
  • Uses LoggingHandler from opentelemetry-instrumentation-logging (not the deprecated SDK one) installed on the root stdlib logger

Changes

  • _provider.py: setup_log_provider() + _create_log_exporter() — mirrors _setup_meter_provider() pattern, same grpc/http protocol switch
  • auto_instrument.py: one setup_log_provider() call wired in after trace setup
  • user-guide.md: new Logging section covering usage, extra={} structured fields, level filtering, trace correlation

Tests

  • Mock unit tests for _create_log_exporter, setup_log_provider, and auto_instrument wiring
  • End-to-end tests with a real in-memory exporter verifying body, severity, resource attributes, extra={} fields, and trace/span correlation — all run in CI without any external dependencies

App usage

No changes needed. After auto_instrument(), all existing logging.getLogger(...) calls ship logs to the OTel backend automatically:

auto_instrument()  # sets up traces, metrics, and logs

logger = logging.getLogger(__name__)
logger.info("Request handled", extra={"tenant_id": tid})

@LucasAlvesSoares
LucasAlvesSoares marked this pull request as ready for review July 28, 2026 16:13
@LucasAlvesSoares
LucasAlvesSoares requested a review from a team as a code owner July 28, 2026 16:13
@jeanscherf jeanscherf added the enhancement New feature or request label Aug 5, 2026
@jeanscherf
jeanscherf force-pushed the feat/otel-standard-logs branch from d24458c to bcb47c9 Compare August 12, 2026 17:10
jeanscherf and others added 13 commits August 13, 2026 13:01
@jeanscherf
feat(telemetry): add OTel standard logs as third signal
0528b12 
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
@jeanscherf
docs(telemetry): document third-party logging library compatibility
5bfe531
@LucasAlvesSoares @jeanscherf
idempotency guard
be9dd93
@LucasAlvesSoares @jeanscherf
Unit tests
8eac3d4
@LucasAlvesSoares @jeanscherf
Unit tests
2980d47
@LucasAlvesSoares @jeanscherf
Attach logger provider if not exists
5f51ccf
@LucasAlvesSoares @jeanscherf
clashing providers
98c7996
@LucasAlvesSoares @jeanscherf
attaching log record processor
9e150dd
@jeanscherf
fix(telemetry): cast get_logger_provider() to SDK type to satisfy ty …
b578f78 
…checker
@jeanscherf
chore: bump version to 0.44.0
990090f
@jeanscherf
fix(telemetry): inject sap.cloud_sdk.* resource attrs when platform o…
37ef724 
…wns LoggerProvider

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.
@jeanscherf
fix(telemetry): eliminate log duplication when platform owns the Logg…
e257972 
…erProvider

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.
@jeanscherf
fix(telemetry): eliminate 4x log duplication with platform auto-instr…
9853613 
…umentation

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.
@jeanscherf
jeanscherf force-pushed the feat/otel-standard-logs branch from 99806b3 to 9853613 Compare August 13, 2026 20:02
LucasAlvesSoares
LucasAlvesSoares previously approved these changes Aug 13, 2026
View reviewed changes

@LucasAlvesSoares LucasAlvesSoares left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

img

@jeanscherf
fix(telemetry): fix CI failures after rebase onto main
db532be 
- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LucasAlvesSoares LucasAlvesSoares LucasAlvesSoares approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jeanscherf @LucasAlvesSoares