OTEL attribute count limit not respected, causing columns dropped#4677
OTEL attribute count limit not respected, causing columns dropped#4677xrmx merged 14 commits intoopen-telemetry:mainfrom
Conversation
889a3ec to
d2ac2c3
Compare
yurishkuro
left a comment
There was a problem hiding this comment.
reverting my approval pending https://github.com/open-telemetry/opentelemetry-python/pull/4677/files#r2198591904
|
Hi @pmcollins , can this change be merged so we can patch our code? |
Hi @giongto35 -- we'll need one of the maintainers to review the change. Once a maintainer has approved, then the change can be merged. In the meantime, I have approved the workflows to run. |
|
@giongto35 avoid merging main unless there are conflicts, doing so just resets the CI checks and wastes maintainers' time as they need to re-approve CI runs. |
|
I see some failures in the checks https://github.com/open-telemetry/opentelemetry-python/pull/4677/checks . Is there anything I need to do to get it merged and @aabmass @ocelotl @lzchen @xrmx please help merge the change if it is ready, it is breaking our production. |
|
@giongto35 please fix linting failures (fine to add annotation to ignore the pylint error) and add a changelog entry |
…en-telemetry#4677) * Allow Passing Limit * Add loglimit to log provider * Add Test from log provider * Fix unused var * Remove UNSET value, only use None as Default value * Update rollback loglimits as arg to loggerprovider * Update test_handler.py * Update CHANGELOG.md --------- Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
…en-telemetry#4677) * Allow Passing Limit * Add loglimit to log provider * Add Test from log provider * Fix unused var * Remove UNSET value, only use None as Default value * Update rollback loglimits as arg to loggerprovider * Update test_handler.py * Update CHANGELOG.md --------- Co-authored-by: Emídio Neto <9735060+emdneto@users.noreply.github.com> Co-authored-by: Riccardo Magliocchetti <riccardo.magliocchetti@gmail.com>
Description
Fixes #4671
This PR fixes a bug where the
LoggingHandlerin the OpenTelemetry Python SDK does not respect theOTEL_ATTRIBUTE_COUNT_LIMITenvironment variable. When this environment variable is set, it should limit the number of attributes in log records, but the LoggingHandler was ignoring this setting entirely and only use the default value 128.It means:
Changes Made
Fixed the Root Cause Bug
The core issue was that
LogRecordwas created with_UnsetLogLimitswhich usedUNSETvalues, and theLogLimits._from_env_if_absentmethod had a bug where it returnedNoneimmediately whenvalue == cls.UNSET, never checking environment variables.Implementation Benefits
This implementation allows users to:
OTEL_ATTRIBUTE_COUNT_LIMIT)Type of change
How Has This Been Tested?
You can run the tests using pytest:
Test Coverage
Added comprehensive tests to verify the fix works correctly:
test_otel_attribute_count_limit_respected_in_logging_handler- Tests that whenOTEL_ATTRIBUTE_COUNT_LIMIT=3, only 3 attributes are kept and the rest are droppedtest_otel_attribute_count_limit_includes_code_attributes- Tests that whenOTEL_ATTRIBUTE_COUNT_LIMIT=5, only 5 attributes are kept and the limit applies to all attribute types including code attributestest_logging_handler_without_env_var_uses_default_limit- Tests that without the environment variable, the default limit of 128 is appliedAll tests pass, confirming the fix works correctly across all these scenarios.
Does This PR Require a Contrib Repo Change?
Checklist: