feat: Add BaggageLogProcessor to opentelemetry-processor-baggage#4371
Conversation
|
Thanks @Manvi2402 for submitting. I think this is a nice analog feature and iiuc shouldn't interfere with the Logging API stabilization efforts. I've left some comments. Please could you also:
|
176f8d3 to
3449a3d
Compare
|
Thanks for the review @tammy-baylis-swi . I've addressed all the comments in the latest commit 3449a3d Added collision handling to avoid overwriting existing log record attributes. |
…s=128, multiple predicates test and README example
|
Thanks for the follow-up @tammy-baylis-swi I've addressed all the comments in the latest commit: Added docstring to on_emit documenting collision handling behavior |
|
Hi @Manvi2402 please add a bit more test coverage for configurable
|
|
Hi @tammy-baylis-swi , I've added the test for max_baggage_attributes. I tried running tox -e precommit and tox -e lint-processor-baggage locally but they fail on Windows because sh command is not available. |
Thanks @Manvi2402 ! I'm not a Windows user myself but I think those tox commands should work if you use git bash or WSL. Could you give one of those a try? |
|
Hi @tammy-baylis-swi , I ran tox -e precommit and it passed after ruff auto-fixed some formatting. I also fixed the pylint issues in log_processor.py (naming convention and staticmethod). The only remaining pylint warning is ALLOW_ALL_BAGGAGE_KEYS in the existing processor.py which was there before my changes. |
MikeGoldsmith
left a comment
There was a problem hiding this comment.
Looks good for me. Thank you @Manvi2402.
Thanks @Manvi2402 , glad you got it working locally. Sorry to sound like a broken record but there's just a few more |
|
Hi @tammy-baylis-swi thank you for your patience and guidance. I've fixed the unsorted imports and missing newlines. |
|
Thanks @Manvi2402 ! And one more from the linting: |
|
This PR has been automatically marked as stale because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 days of this comment. |
Replaced Apache License comments with SPDX identifier.
Replaced Apache License header with SPDX identifier.
|
Thanks! |
Automated update of OpenTelemetry dependencies. **Build Status:** ❌ [failure](https://github.com/aws-observability/aws-otel-python-instrumentation/actions/runs/27184408784) **Updated versions:** - [OpenTelemetry Python](https://github.com/open-telemetry/opentelemetry-python/releases/tag/v1.42.1): 1.42.1 - [OpenTelemetry Contrib](https://github.com/open-telemetry/opentelemetry-python-contrib/releases/tag/v0.63b1): 0.63b1 - [opentelemetry-sdk-extension-aws](https://pypi.org/project/opentelemetry-sdk-extension-aws/2.1.0/): 2.1.0 - [opentelemetry-propagator-aws-xray](https://pypi.org/project/opentelemetry-propagator-aws-xray/1.0.2/): 1.0.2 **Upstream releases with breaking changes:** Note: the mechanism to detect upstream breaking changes is not perfect. Be sure to check all new releases and understand if any additional changes need to be addressed. **opentelemetry-python-contrib:** - [Version 1.41.0/0.62b0](https://github.com/open-telemetry/opentelemetry-python-contrib/releases/tag/v0.62b0) *Description of changes:* - Drops the `opentelemetry-instrumentation-boto` runtime dependency (legacy boto2 instrumentation), removed entirely upstream in: open-telemetry/opentelemetry-python-contrib#4303 - Drops Python 3.9 support to match upstream's new minimum (Python ≥3.10): open-telemetry/opentelemetry-python#5076 (core) and open-telemetry/opentelemetry-python-contrib#4412 (contrib) - Switches `from importlib_metadata import version` to stdlib `from importlib.metadata import version` after upstream dropped the third-party `importlib-metadata` package from `opentelemetry-api`: open-telemetry/opentelemetry-python#3234 - Refactors the botocore patches (`_botocore_patches.py`) to use the new `ExtensionRegistry` API. Upstream split `_KNOWN_EXTENSIONS` into `_BOTOCORE_EXTENSIONS` + `_AIOBOTOCORE_EXTENSIONS`, removed `_find_extension`, and moved `_safe_invoke` to `opentelemetry.instrumentation.botocore.utils` as part of adding aiobotocore instrumentation: open-telemetry/opentelemetry-python-contrib#4049 - Removes the Starlette ASGI `suppress_http_instrumentation` patch (which the existing ADOT TODO already flagged) since the ASGI middleware now natively honors `suppress_http_instrumentation` upstream: open-telemetry/opentelemetry-python-contrib#4375 - Adapts the configurator tests to upstream's renamed `BaggageSpanProcessor._baggage_key_predicate` → `_predicates` (now a list of callables, after the constructor was reworked to accept either a single predicate or a sequence): open-telemetry/opentelemetry-python-contrib#4371 - Adapts `test_otlp_attribute_vs_body_types` to upstream's `_VALID_ANY_VALUE_TYPES` switching `typing.Mapping`/`typing.Sequence` to `collections.abc.Mapping`/`collections.abc.Sequence`: open-telemetry/opentelemetry-python#5133 - Bumps the mysqlclient contract-test pin from `2.2.4` to `2.2.8` and updates the matching `db.name` assertion. The 0.62b0 dbapi semantic-convention migration (open-telemetry/opentelemetry-python-contrib#4109) added an empty-string filter inside `_set_db_name`, which exposed a latent gap: mysqlclient < 2.2.7 didn't expose a Python-level `conn.db` attribute (added upstream in PyMySQL/mysqlclient#752, shipped in mysqlclient 2.2.7), so OTel's dbapi instrumentation defaulted `database` to `""` and the empty value used to land on the span unconditionally. After 0.62b0 the empty value is dropped, so the assertion `db.name == ""` now fails with "key not found". Bumping mysqlclient lets `conn.db` flow through as the actual database name (`testdb`) so the assertion becomes meaningful. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. --------- Co-authored-by: github-actions <[email protected]> Co-authored-by: Steve Liu <[email protected]>
Description
Adds
BaggageLogProcessorwhich reads entries stored in Baggage from the current context and adds the baggage entries' keys and values to the log record as attributes on emit.This is analogous to the existing
BaggageSpanProcessorbut for logs.Fixes #4062
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Added unit tests in
test_baggage_log_processor.py:test_check_the_baggage- verifies BaggageLogProcessor is instance of LogRecordProcessortest_baggage_added_to_log_record- verifies baggage is added to log attributestest_baggage_with_prefix- verifies predicate filtering with string prefixtest_baggage_with_regex- verifies predicate filtering with regextest_no_baggage_not_added- verifies no baggage keys added when baggage is emptyDoes This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.