-
Notifications
You must be signed in to change notification settings - Fork 612
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
added readme.rst file log record enrichment behavior of logging instr… #2904
base: main
Are you sure you want to change the base?
added readme.rst file log record enrichment behavior of logging instr… #2904
Conversation
…umentation Signed-off-by: Wali, Sunil Shidrayi <[email protected]>
Hi @lzchen , Could you please review and approve . |
Log record enrichment behavior of logging instrumentation | ||
|
||
""" | ||
The OpenTelemetry ``logging`` integration automatically injects tracing context into log statements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use instrumentation
as the component name
|
||
""" | ||
The OpenTelemetry ``logging`` integration automatically injects tracing context into log statements. | ||
It's structured within an `if` statement that checks [`set_logging_format`] is `True`, indicating that logging configuration should be applied. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think implementation details like this is necessary to include in the README
The OpenTelemetry ``logging`` integration automatically injects tracing context into log statements. | ||
It's structured within an `if` statement that checks [`set_logging_format`] is `True`, indicating that logging configuration should be applied. | ||
|
||
The integration registers a custom log record factory with the the standard library logging module that automatically inject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
tracing context into log record objects. Optionally, the integration can also call ``logging.basicConfig()`` to set a logging | ||
format with placeholders for span ID, trace ID and service name. | ||
|
||
1. **Determining the Log Format**: The code first attempts to determine the logging format by looking for a `logging_format` key in the [`kwargs`]( dictionary. If it's not found, it tries to fetch the value from an environment variable named [`OTEL_PYTHON_LOG_FORMAT`]. If neither is available, it defaults to a predefined constant [`DEFAULT_LOGGING_FORMAT`]. This approach provides flexibility, allowing the log format to be specified through function arguments, environment variables, or falling back to a default if neither is provided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this level of detail for implementation. I believe it is enough to have the related env vars included in the README (as you have below).
|
||
|
||
|
||
API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put the API section as the first section before env vars
LoggingInstrumentor().instrument(set_logging_format=True) | ||
|
||
|
||
Note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note | |
Setting logging format outside of the instrumentation |
|
||
The default value is ``info``. | ||
|
||
Options are: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to mention these as these are standard python logging library values.
LoggingInstrumentor(log_level=logging.DEBUG) | ||
|
||
|
||
The default value is ``info``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value is ``info``. | |
The default value is ``logging.INFO``. |
|
||
.. code-block:: | ||
|
||
{default_logging_format} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to have a separate section for the default format. Simply paste it into the below section regarding OTEL_PYTHON_LOG_FORMAT
env var
|
||
.. code-block:: | ||
|
||
{default_logging_format} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to link out, just paste the format here.
|
||
.. envvar:: OTEL_PYTHON_LOG_CORRELATION | ||
|
||
This env var must be set to ``true`` in order to enable trace context injection into logs by calling ``logging.basicConfig()`` and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This env var must be set to ``true`` in order to enable trace context injection into logs by calling ``logging.basicConfig()`` and | |
This env var must be set to ``true`` in order to enable trace context injection into logs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't include implementation detail.
instrumentation/opentelemetry-instrumentation-logging/README.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Leighton Chen <[email protected]>
Fixes #2642