Add Structlog Handler as Instrumentation#4286
Add Structlog Handler as Instrumentation#4286JWinermaSplunk wants to merge 4 commits intoopen-telemetry:mainfrom
Conversation
JWinermaSplunk
changed the title
JWinermaSplunk
force-pushed
the
structlog-handler
branch
from
e0317fa to
fab1022
Compare
| def std_to_otel(levelno: int) -> SeverityNumber: | ||
| """ | ||
| Map python log levelno to OTel log severity number. | ||
| """ |
There was a problem hiding this comment.
This duplicates some of logging instrumentation. What about moving reusable helpers to opentelemetry-instrumentation at some point?
There was a problem hiding this comment.
Understood, would this be a good thing to expose in this PR or should we open a separate one?
There was a problem hiding this comment.
This PR might be a good time to introduce fewer lines overall.
| return _STD_TO_OTEL[levelno] | ||
|
|
||
|
|
||
| class StructlogHandler: |
There was a problem hiding this comment.
Could this be simplified by subclassing an existing type in structlog (which I don't know that well 🙂 )?
|
|
||
| class StructlogHandler: | ||
| """ | ||
| A structlog processor that translates structlog events into OpenTelemetry LogRecords. |
There was a problem hiding this comment.
Nit: the docstrings could be more consistent in referring to this as a handler instead of a processor, the latter in my mind being something like SimpleLogRecordProcessor.
|
Thank you for undertaking this! |
| "instrumentation": "opentelemetry-instrumentation-starlette==0.62b0.dev", | ||
| }, | ||
| { | ||
| "library": "structlog ~= 21.1", |
There was a problem hiding this comment.
>= 21.1 to match the version constraint in package.py?
| current_processors.insert(insert_position, processor) | ||
|
|
||
| # Reconfigure structlog with the new processor chain | ||
| structlog.configure(processors=current_processors) |
There was a problem hiding this comment.
During auto-instrumentation, instrumentors will run first, then the user's code will run, and if that code calls structlog.configure, looks like it will overwrite this config. The solution may be to monkeypatch structlog.configure.
# Conflicts: # .github/workflows/test_0.yml # .github/workflows/test_1.yml # .github/workflows/test_2.yml # opentelemetry-contrib-instrumentations/pyproject.toml
# Conflicts: # .github/workflows/test_1.yml # .github/workflows/test_2.yml # .github/workflows/test_3.yml
JWinermaSplunk
force-pushed
the
structlog-handler
branch
from
fd07c37 to
ddae8b5
Compare
3 participants
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes open-telemetry/opentelemetry-python#2993
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.