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

OpenTelemetryLayer to make use of Filtered #15

Open
BrynCooke opened this issue Apr 16, 2023 · 3 comments
Open

OpenTelemetryLayer to make use of Filtered #15

BrynCooke opened this issue Apr 16, 2023 · 3 comments

Comments

@BrynCooke
Copy link

BrynCooke commented Apr 16, 2023

Feature Request

Motivation

It seems that OpenTelemetryLayer may be able to make use of Filtered to allow Registry to avoid creating spans in some cases.

For the vast majority of cases users will set their otel sampling rate to something low, but this doesn't have an effect on the tracing side of things, and seems to have quite a performance hit.

Otel is complicated, and I am very aware that I may have missed the reason as to why this is not feasible while remaining compatible with the otel spec.

Proposal

Add a new function OpenTelemetryLayer::prefiltered(self) that returns the layer wrapped via with_filter.
Move the logic for deciding if a span is of interest into the Filter. Only apply the logic for root spans.

The downside is that the tracer would have to be moved to an Arc so that it can be shared between the filter function and the layer.

I can create a PR POC if the above seems reasonable.

Alternatives

To avoid the arc, modify the FilterLayer to allow self filtering if a layer implements Filtered.

@BrynCooke BrynCooke changed the title OpenTelemetryLayer implement Filtered OpenTelemetryLayer to make use of Filtered Apr 16, 2023
@davidbarsky
Copy link
Member

Hmm, I'm not entirely sure why this strictly needed as opposed to writing tracing_opentelemetry::layer().with_filter(filter).with_tracer(tracer).

(I'm genuinely unsure, so I'd love to get clarification!)

@BrynCooke
Copy link
Author

I'm not really explaining this well.

I'm assuming that we are on the same page regarding the intent, that the filter would take on the work that is currently inside the otel layer to call sampled_context. Calling of sampled_context would no longer be in the remit of the otel layer itself.

Then from the user point of view I'd like to be able to call:
tracing_opentelemetry::layer().with_tracer(tracer)
And have it internally sort out the details of wrapping the otel layer in a filter for performance.

However this would probably be a breaking change, so maybe some different api or prefiltered method on otel layer could be in order.

@BrynCooke
Copy link
Author

Looks like https://docs.rs/tracing-subscriber/latest/tracing_subscriber/layer/trait.Layer.html#method.on_layer can be used to add the filter so no new APIs would be needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants