-
Notifications
You must be signed in to change notification settings - Fork 806
Support passing tracer provider in aiohttp server instrumentation lib #3819
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
|
@bisgaard-itis You need to sign the CLA in order to contribute to OpenTelemetry, thanks. |
Perfect, thanks a lot for pointing this out @xrmx. I have signed it. |
|
@krnr this PR has been hanging for some time. Can I convince you to to review it? 🙏🏻 |
i see no issues, the code is pretty much straight-forward - the same middleware logic, but the middleware itself is created on-the-fly. i like it 👍 |
...-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py
Show resolved
Hide resolved
...-instrumentation-aiohttp-server/src/opentelemetry/instrumentation/aiohttp_server/__init__.py
Outdated
Show resolved
Hide resolved
|
Also:
|
…lemetry#3875) * Only decode JSON input buffer in Anthropic Claude streaming _decode_tool_use was only used when _tool_json_input_buf was found, but we were decoding the entire _content_block after adding _tool_json_input_buf to it. The _content_block overall which could contain non-JSON elements (e.g. {}), causing failures. To fix this, we have removed _decode_tool_use helper function and inlined JSON decoding logic directly into content_block_stop handler in _process_anthropic_claude_chunk, where we only use it to decode _tool_json_input_buf before appending to _content_block. * Update test_botocore_bedrock.py Fix lint: `opentelemetry-instrumentation-botocore/tests/test_botocore_bedrock.py:2990:4: C0415: Import outside toplevel (opentelemetry.instrumentation.botocore.extensions.bedrock_utils.InvokeModelWithResponseStreamWrapper) (import-outside-toplevel)` * Update test_botocore_bedrock.py Remove extra line * fix lint issue
* Stop using deprecated span.instrumentation_info * aiohttp-client: add support for OTEL_PYTHON_EXCLUDED_URLS / OTEL_PYTHON_HTTPX_EXCLUDED_URLS * Add docs * Add changelog * Please lint * Update CHANGELOG.md Co-authored-by: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com> * Test for both env vars * Assert at each iteration --------- Co-authored-by: Tammy Baylis <96076570+tammy-baylis-swi@users.noreply.github.com>
a5bada0 to
4a57b3e
Compare
|
Thanks a lot for the review @tammy-baylis-swi 🙏🏻. I have done the changes you requested. |
…acerProvider-in-aiohttp-server-instrumentation-lib
| from opentelemetry.sdk.trace.sampling import ParentBased, TraceIdRatioBased | ||
| AioHttpServerInstrumentor().instrument() | ||
| resource = Resource(attributes={"service.name": "my-aiohttp-service"}) |
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.
Great! I would also add a comment so newer/quickstart users know that these are optional:
| resource = Resource(attributes={"service.name": "my-aiohttp-service"}) | |
| # Optional: configure non-default TracerProvider, resource, sampler | |
| resource = Resource(attributes={"service.name": "my-aiohttp-service"}) |
|
For the current pypy test failure in CI/CD, a couple of the definitions like The GenAI test fail is unrelated to the changes in this PR and will be patched by the maintainers. |
Description
Support passing a
TracerProviderwhen using the aiohttp server instrumentation library. This gives users of the library the possibility to provide their own TracerProvider instead of relying on the global one. This is in particular useful for testing.Fixes # (issue)
TracerProviderwhen instrumenting an aiohttp server #3801Type 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
test_non_global_tracer_providerwhich checks that a non-globalTracerProvidercan be provided when instrumenting the aiohttp server. I check that the non-globalTracerProvideris used by the instrumentation library by setting a non-default sampling probability in that provider and checking that the corresponding number of traces are collected.Does This PR Require a Core Repo Change?
Checklist:
See contributing.md for styleguide, changelog guidelines, and more.