docs: update logs example to use opentelemetry-instrumentation-logging#5344
docs: update logs example to use opentelemetry-instrumentation-logging#5344avinab-neogy wants to merge 3 commits into
Conversation
|
|
| OpenTelemetry Logs SDK | ||
| ====================== | ||
|
|
||
| .. warning:: |
There was a problem hiding this comment.
This is unrelated to the LoggingHandler
There was a problem hiding this comment.
Sorry about that, I misread the issue scope. Having re-read #4738, the warning removal was out of scope, restored it. The PR now only documents the LoggingHandler move.
|
I think your changes are not very relevant with #4738 but the changes regarding the LoggingHandler move are good |
| logger1 = logging.getLogger("myapp.area1") | ||
| logger2 = logging.getLogger("myapp.area2") | ||
|
|
||
| # Ensure log records propagate to root logger so the handler picks them up |
There was a problem hiding this comment.
Are these really needed? Would be nice to test the examples
There was a problem hiding this comment.
Tested this end to end with the collector, all 6 log records came through fine. I have added the explicit propagate=True based on this from #4738:
only the root logger is instrumented, so seen loggers must have propagate set to True
if you are loading logging settings with logging/.config.dictConfig, you must save the root loggers before applying
dictConfig
Also tested locally that dictConfig with a root section does wipe the OTel handler completely. The explicit assignment is hence there as a reminder for users who might use dictConfig. Also happy to just keep it as a comment if you prefer.
987a3af to
9258631
Compare
Description
Update the logs example documentation to reflect that
LoggingHandlerhas moved from the SDK to the contrib package.
Refs #4738
Changes made:
opentelemetry-instrumentation-loggingLoggingHandlerhas moved toopentelemetry-instrumentation-logging(deprecated in opentelemetry-sdk: deprecate logging handler #4919)propagate=Truewith explanation comment inexample.pyType of change
How Has This Been Tested?
tox -e docs— build succeeded with no warningstox -e spellcheck— passedDoes This PR Require a Contrib Repo Change?
Checklist: