-
Notifications
You must be signed in to change notification settings - Fork 607
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved logging instrumentor #2718
Open
Olegt0rr
wants to merge
12
commits into
open-telemetry:main
Choose a base branch
from
Olegt0rr:refactor-logging
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-logging # Conflicts: # CHANGELOG.md
xrmx
reviewed
Jul 23, 2024
.../opentelemetry-instrumentation-logging/src/opentelemetry/instrumentation/logging/__init__.py
Outdated
Show resolved
Hide resolved
Would you be able to add a more informative description to the change that this pr pertains? A sample showing it in action and the expected output would be great as well. |
Updated PR description |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Logging fields customization
Having connected this tool, I was surprised that the names of the logging fields were hard-coded. The naming convention for such fields in my company is different, so I couldn't use this module as it is. I decided that I could easily inherit and replace the fields with my own, but when I opened the code, I was surprised that all the action of the instrumentator is in one function and it will not be possible to change the behavior with a little modification.
As a result, I decided to add the ability to specify my field names to the instrumentator, while not changing the standard behavior (for those who already use this instrumentator as is).
As I added in the documentation, it is now possible to specify any convenient names for the fields:
Performance
LogRecordFactory without
log_hook
Now, for cases when a
log_hook
is not passed to the instrumentator, a factory is passed to logging, which does not check the possibility of usinglog_hook
every log record.Single
service_name
callNow we don't calculate
service_name
on log record, get it once - on instrumentation.Typehints
Added type hints for every function signature and class variable
Code readability
Code style before change was like this:
Now it's used early log record return for every case, when you can't get trace or span:
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.