Skip to content

LoggingInstrumentation: Set Handler Format Explictly If basicConfig Has Already Been Called #1939

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

LoggingInstrumentation: Set Handler Format Explictly If basicConfig Has Already Been Called #1939

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68687f2
Add ability to set log format for apps that have already had basicCon…
rjduffner Sep 7, 2023
7242297
Merge branch 'main' into loggingBasicConfig
ocelotl Apr 28, 2024
2d1e863
Merge branch 'main' into loggingBasicConfig
ocelotl May 1, 2024
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
7 changes: 6 additions & 1 deletion ...entelemetry-instrumentation-logging/src/opentelemetry/instrumentation/logging/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,12 @@ def record_factory(*args, **kwargs):
)
log_level = log_level or logging.INFO

logging.basicConfig(format=log_format, level=log_level)
root_logger = logging.getLogger()
if root_logger.hasHandlers():
for handler in root_logger.handlers:
handler.setFormatter(logging.Formatter(log_format))
Copy link
Contributor

@ocelotl ocelotl May 1, 2024

Choose a reason for hiding this comment

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

What about log_level?

Copy link
Contributor

@dhofstetter dhofstetter May 1, 2024

Choose a reason for hiding this comment

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

@ocelotl Is there a reason to review this pr and not the more recent one here?

Copy link
Contributor Author

@rjduffner rjduffner May 11, 2024

Choose a reason for hiding this comment

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

@ocelotl, as @dhofstetter, said lets go with their newer and more complete pr. i meant this as more a discussion starter.

thanks for the review though.

also ill close this pr so its not confusing anymore

else:
logging.basicConfig(format=log_format, level=log_level)

def _uninstrument(self, **kwargs):
if LoggingInstrumentor._old_factory:
Expand Down