-
Notifications
You must be signed in to change notification settings - Fork 783
Add Labeler and instrumentation integration for custom attribute addition to HTTP request duration metrics (Flask, Django, Falcon, WSGI, ASGI) #3689
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
base: main
Are you sure you want to change the base?
Conversation
d676f55
to
6cec2f1
Compare
6cec2f1
to
ce12b1b
Compare
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 would be nice to have 🙌🏻
opentelemetry-instrumentation/src/opentelemetry/instrumentation/_labeler/_internal/__init__.py
Outdated
Show resolved
Hide resolved
opentelemetry-instrumentation/src/opentelemetry/instrumentation/_labeler/_internal/__init__.py
Outdated
Show resolved
Hide resolved
opentelemetry-instrumentation/src/opentelemetry/instrumentation/_labeler/_internal/__init__.py
Show resolved
Hide resolved
opentelemetry-instrumentation/src/opentelemetry/instrumentation/_labeler/_internal/__init__.py
Outdated
Show resolved
Hide resolved
key: The attribute key | ||
value: The attribute value (must be a primitive type) | ||
""" | ||
with self._lock: |
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.
If I remove the locks, test_thread_safety doesn't fail, are we missing asserts?
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 did add one new test and updated the existing test in 0bfd192, not sure if that helps.
I tried the same (comment out all with self._lock
) and tried adding other tests, but none of them would fail. I'm not sure if there is a way? The tests should be using real, concurrent threads but without something like writing a C extension I don't think GIL can be released.
Description
Adds new a 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 request context. Theadd
/add_attributes
can be called by user's instrumented code manually, or by custom distros. The following instrumentors have been updated in this PR so that custom attributes are read from the Labeler in context, then merged into the other attributes (new or old semconv) that are passed to therecord
calls of the request duration histograms:opentelemetry-instrumentation-flask
opentelemetry-instrumentation-django
opentelemetry-instrumentation-falcon
opentelemetry-instrumentation-wsgi
opentelemetry-instrumentation-asgi
Example OTel Collector debug metrics from ASGI OpenTelemetryMiddleware instrumentation, with histograms including custom attributes: https://gist.github.com/tammy-baylis-swi/974ec71d5fb9e88968327b175f2b59a5
Addresses #3702, #3703
Type of change
Please delete options that are not relevant.
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
Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.