(Redo) Add experimental Labeler to store custom attributes in OTel Context

[tammy-baylis-swi]

Description

Adds experimental Python instrumentation Labeler, inspired by Go's net/http instrumentation: open-telemetry/opentelemetry-go-contrib#306. It stores KVs added to it in the current OTel context. The add / add_attributes can be called by user's instrumented code manually or by custom distros. enrich_metric_attributes applies added KVs to existing attributes; contrib instrumentors can be updated to use this before metrics record/add/etc.

Fixes #3702

Replaces #3689. Better utilizes OTel API Context. Shrinks scope to adding util only first.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

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

• tox -e py312-test-opentelemetry-instrumentation

Does This PR Require a Core Repo Change?

• Yes. - Link to PR:
• No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

• Followed the style guidelines of this project
• Changelogs have been updated
• Unit tests have been added
• Documentation has been updated

[herin049]

I'm not sure if this is what the context API is designed for, but we could look into potentially integrating the labeler into the context API (similar to baggage). Maybe something like this:

from opentelemetry.instrumentation._labeler._internal import create_labeler, get_labeler

labeler_ctx = create_labeler()
token = context.attach(labeler_ctx)

try:
    # call request handler code
    ... 
finally:
    context.detach(token)

...

def handler(...):
    labeler = get_labeler()
    if labeler:
        labeler.set_attribute("foo", "bar")

...

[herin049]

Furthermore, for the get_labeler() function, we have have it optionally return a DummyLabeler that still allows you to call set_attribute, but won't end up being recorded if no labeler was originally attached to the current context.

[herin049]

Looking at the Go implementation, it also looks like they do attach the labeler to the current context in a similar manner to what I have suggested. Let me know what you think.

[tammy-baylis-swi]

Hi @herin049 ! 👋 Sorry for delay, was busy

potentially integrating the labeler into the context API (similar to baggage)

Looking at the Go implementation, it also looks like they do attach the labeler to the current context

It would be good to be consistent with Go. I also think this current implementation keeps more of the responsibility in the Labeler itself so the instrumentations only have to get the Labeler and enrich with its metrics, e.g. this WIP for ASGI/WSGI adoption: https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4300/changes#diff-7eb6c4565bcdedf2510f8932a48d5781f49bdf50b38a56d7d115a76d2100e186 So while it's a bit of a departure from Go, instrumentors and users don't have to do explicit attach/detach themselves.

[herin049]

Hi @herin049 ! 👋 Sorry for delay, was busy

potentially integrating the labeler into the context API (similar to baggage)

Looking at the Go implementation, it also looks like they do attach the labeler to the current context

It would be good to be consistent with Go. I also think this current implementation keeps more of the responsibility in the Labeler itself so the instrumentations only have to get the Labeler and enrich with its metrics, e.g. this WIP for ASGI/WSGI adoption: https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4300/changes#diff-7eb6c4565bcdedf2510f8932a48d5781f49bdf50b38a56d7d115a76d2100e186 So while it's a bit of a departure from Go, instrumentors and users don't have to do explicit attach/detach themselves.

Thanks! It was only a suggestion, I'm happy deviating from the Go implementation.

The reason I suggested the context approach is that it would handle scenarios where you have multiple active labelers more gracefully. With this implementation, setting or clearing a labeler destroyed the original labelers. However, this is likely not a very common use case.

If you're concerned about the inconvenience with working directly with the context API, we could always introduce a context manager for convenience.

[tammy-baylis-swi]

Hi @herin049 ! 👋 Sorry for delay, was busy

potentially integrating the labeler into the context API (similar to baggage)

Looking at the Go implementation, it also looks like they do attach the labeler to the current context

It would be good to be consistent with Go. I also think this current implementation keeps more of the responsibility in the Labeler itself so the instrumentations only have to get the Labeler and enrich with its metrics, e.g. this WIP for ASGI/WSGI adoption: https://github.com/open-telemetry/opentelemetry-python-contrib/pull/4300/changes#diff-7eb6c4565bcdedf2510f8932a48d5781f49bdf50b38a56d7d115a76d2100e186 So while it's a bit of a departure from Go, instrumentors and users don't have to do explicit attach/detach themselves.

Thanks! It was only a suggestion, I'm happy deviating from the Go implementation.

The reason I suggested the context approach is that it would handle scenarios where you have multiple active labelers more gracefully. With this implementation, setting or clearing a labeler destroyed the original labelers. However, this is likely not a very common use case.

If you're concerned about the inconvenience with working directly with the context API, we could always introduce a context manager for convenience.

I appreciate it! I think I'd like to keep the current overall approach for this one.

[emdneto]

all my comments from the previous PR were addressed, and the current implementation looks solid to me. We can review it anytime later if needed. Thanks @tammy-baylis-swi