Skip to content
Closed
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
1 change: 1 addition & 0 deletions .changelog/5420.changed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
docs: warn about `dictConfig` clearing OTel handlers in logs example
22 changes: 22 additions & 0 deletions docs/examples/logs/README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,28 @@ The source files of these examples are available :scm_web:`here <docs/examples/l
``opentelemetry-instrumentation-logging`` package in the contrib repo
and is deprecated in ``opentelemetry-sdk``.

.. warning::
If you configure logging using ``logging.config.dictConfig``, be aware
that it may clear existing handlers on the root logger, including the
OTel handler. To avoid losing telemetry, save and restore the root
logger handlers around the ``dictConfig`` call:

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@DylanRussell @herin049 Thanks for pointing that out, you're both totally right!

To answer your question @herin049, I went back into my local setup and my original test was just running standard Python's dictConfig function with a vanilla StreamHandler, completely bypassing the OTel instrumentation.

I tested it again using the OpenTelemetry library, and I could see how it automatically saves and restores the handler.

import logging
import logging.config
from opentelemetry.instrumentation.logging import LoggingInstrumentor

LoggingInstrumentor().instrument()
root_logger = logging.getLogger()

print('Before dictConfig:', root_logger.handlers) # [<LoggingHandler (NOTSET)>]

logging.config.dictConfig({'version': 1, 'root': {'level': 'DEBUG', 'handlers': []}})

print('After dictConfig: ', root_logger.handlers) # [<LoggingHandler (NOTSET)>] - It does not get cleared!

That said, I originally opened this to address Point 3 from Issue #4738 :

  1. the root logger is instrumented, so handlers on it must not be cleared and if you are loading logging settings with logging/.config.dictConfig, you must save the root loggers before applying dictConfig, then re-apply missing handlers

This issue explicitly asked for this warning to be added to the docs. I'm guessing that requirement came from someone configuring the LoggingHandler manually via the core SDK, where this monkey-patch doesn't exist?

How would you prefer to handle this? I'm happy to do either of the following:

  1. Close this PR, and we can update Point 3 on the original issue to say it's no longer an issue when using the instrumentation library.
  2. Keep the PR open but rewrite the warning to clarify that it only applies when configuring the LoggingHandler manually via the core SDK, since dictConfig will still clear their handlers without the instrumentation package.

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.

I think we should just close this PR, I don't see any behavior here that needs to be documented.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! Closing this PR out. Thanks for the review and for clearing this up!

.. code-block:: python

import logging
import logging.config

# Save OTel handlers before calling dictConfig
otel_handlers = logging.getLogger().handlers[:]

logging.config.dictConfig({
"version": 1,
"root": {"level": "DEBUG"},
})

# Re-apply saved handlers
for handler in otel_handlers:
logging.getLogger().addHandler(handler)

Installation
------------
Expand Down
Loading