Skip to content
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

contrib psr18 auto-instrumentation also instruments exporter #878

Closed
brettmc opened this issue Dec 1, 2022 · 3 comments · Fixed by #880
Closed

contrib psr18 auto-instrumentation also instruments exporter #878

brettmc opened this issue Dec 1, 2022 · 3 comments · Fixed by #880
Assignees
Labels
bug Something isn't working instrumentation Issues that relate to instrumentation for OpenTelemetry PHP

Comments

@brettmc
Copy link
Collaborator

brettmc commented Dec 1, 2022

Whilst doing some initial testing of the new psr-18 auto-instrumentation, with PHP 8.1 + otel dev-main, using any exporter which uses PsrTransport (everything except grpc), I noticed that it also creates spans/traces for the exporting call.

If using batch processor, that's one extra trace exported (because it happens as part of shutdown, when there is no active context).
If using simple processor, it's pathological: each span exported creates a new http request which is also exported and traced. I didn't let it continue (one trace contained 27k spans, so it would probably only stop on resource exhaustion).

First trace is with simple span exporter (request aborted after some time), second two are with batch span exporter:
psr18-simple-vs-batch

Detail of simple span exporter's trace:
psr18-simple-detailed

Proposed solution:
Create a generic way to be able to identify psr-7 requests which are exporting. My best idea so far is to add a known header such as X-Is-Otel-Exporter from PsrTransport. HTTP auto-instrumentations such as psr-18 can check for this header, and ignore it (and drop the header before it is sent), essentially a pseudo-attribute for psr-7 requests.

@brettmc brettmc added the bug Something isn't working label Dec 1, 2022
@brettmc brettmc self-assigned this Dec 1, 2022
@brettmc brettmc added the instrumentation Issues that relate to instrumentation for OpenTelemetry PHP label Dec 1, 2022
@pdelewski
Copy link
Member

pdelewski commented Dec 1, 2022

I don't see better solution for now. Is dropping this header necessary or it's just optimization?

@brettmc
Copy link
Collaborator Author

brettmc commented Dec 1, 2022

I don't see better solution for now. Is dropping this header necessary or it's just optimization?

It's probably not necessary to drop it, I might ask on the maintainers list whether this has come up in other SIGs. Dropping does seem unnecessary, since if there wasn't an auto-instrumentation to check/drop, it would be sent to the the collector/other endpoint.

@Nevay
Copy link
Contributor

Nevay commented Dec 1, 2022

IMO we should run span processor flush and metric reader collect with the Context that was active during creation (ref).
The Globals initializer already activates noop implementations before invoking the initializers to support this / to disable auto-instrumentation of SDK requests etc. by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working instrumentation Issues that relate to instrumentation for OpenTelemetry PHP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants