Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Add ability to filter and defer spans #816

Closed
wants to merge 1 commit into from

Conversation

sigix
Copy link

@sigix sigix commented Dec 3, 2021

Signed-off-by: Edward Bross edward.bross@appian.com

Which problem is this PR solving?

Short description of the changes

  • adds the ability to reduce the number of spans reported by setting a filter and a defer threshold
  • implemented via a new FilteringReporter that handles the filtering and buffering logic before forwarding spans to a delegate

@sigix sigix force-pushed the filtering-reporter branch from f0da153 to aeaa6c4 Compare December 20, 2021 13:13
@yurishkuro
Copy link
Member

I prefer not to merge this because this sort of naive filtering can easily lead to broken traces, it is not particularly general as a mechanism, and in the current form can be implemented as an extension instead of being part of the core lib.

Cc @pavolloffay @jpkrohling - thoughts?

@jpkrohling
Copy link
Collaborator

There's some discussion about filtering in general here: open-telemetry/opentelemetry-collector-contrib#6341

In this particular case, I see how it could be useful to filter out spans quicker than a specific threshold, but I have the feeling that this is too little: users will want a way to filter spans based on other characteristics. According to our deprecation plan, we are not accepting new features in about one week, so, there won't be a way to do any other type of filtering.

If this comes with good and clear documentation that this is useful to specific use cases and that there are known side-effects (perhaps with concrete examples), I'd be OK in accepting this feature. Otherwise, I think it might cause more harm than good.

Signed-off-by: Edward Bross <edward.bross@appian.com>
@sigix sigix force-pushed the filtering-reporter branch from aeaa6c4 to 90c3f14 Compare December 30, 2021 21:35
@sigix
Copy link
Author

sigix commented Dec 30, 2021

@jpkrohling I added a section to jaeger-core README.md with documentation and a use case.

@yurishkuro - using OpenTelemetry, I can see how to implement this via extension instead of modification, but it was not evident to me how to do the same with this library -- the configuration of the reporter seemed to be completely internal to the Configuration class with no accessible extension points. If there is a way to do this via extension, that would be just fine.

@yurishkuro
Copy link
Member

There's no configuration based extension mechanism, but you can always instantiate a custom reporter and pass it to the tracer.

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

Successfully merging this pull request may close these issues.

Deferred span reporting
3 participants