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

[processor/tailsamplingprocessor] Policy on span_count & min_spans #18215

Closed
yehaotian opened this issue Feb 1, 2023 · 12 comments
Closed

[processor/tailsamplingprocessor] Policy on span_count & min_spans #18215

yehaotian opened this issue Feb 1, 2023 · 12 comments
Assignees
Labels
enhancement New feature or request processor/tailsampling Tail sampling processor

Comments

@yehaotian
Copy link

yehaotian commented Feb 1, 2023

Component(s)

processor/tailsamplingprocessor - span_count

Is your feature request related to a problem? Please describe.

For now tail sampling processor policy on span_count only have min_spans to sample on traces contains minimal number of spans.
It is good to have max_spans to sample traces contains maximum number of spans, to help filter out giant traces. Usually we find out the giant traces are not helpful and takes a lot of data volume quota.

Describe the solution you'd like

Something similar

policies:
      [
# sample traces with maximum 20 spans.
         {
            name: test-policy-10,
            type: span_count,
            span_count: {max_spans: 20}
         },
# or not sample traces with more than 20 spans
         {
            name: test-policy-11,
            type: span_count,
            span_count: {min_spans: 20, invert_match: true}
         },

...
      ]

Describe alternatives you've considered

No response

Additional context

No response

@yehaotian yehaotian added enhancement New feature or request needs triage New item requiring triage labels Feb 1, 2023
@github-actions github-actions bot added the processor/tailsampling Tail sampling processor label Feb 1, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 1, 2023

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mizhexiaoxiao
Copy link
Contributor

Seems reasonable to me.
Assign me if possible, i should be able to work on it

@atoulme atoulme removed the needs triage New item requiring triage label Feb 2, 2023
@atoulme
Copy link
Contributor

atoulme commented Feb 2, 2023

@mizhexiaoxiao it's yours!

@yehaotian
Copy link
Author

Thanks for the help!

@jpkrohling
Copy link
Member

# sample traces with maximum 20 spans.
         {
            name: test-policy-10,
            type: span_count,
            span_count: {max_spans: 20}
         },
# or not sample traces with more than 20 spans
         {
            name: test-policy-11,
            type: span_count,
            span_count: {min_spans: 20, invert_match: true}
         },

Aren't those equivalent? Why not just have one of min_spans/max_spans? Or remove the invert match?

@yehaotian
Copy link
Author

yehaotian commented Feb 2, 2023

I mentioned or in the comment, either way works for me

@jpkrohling
Copy link
Member

jpkrohling commented Feb 2, 2023

I think just a "num_spans" or just "num" should be fine. I know we use invert_match elsewhere, but perhaps we could use a different name? Like, operation=min?

@mizhexiaoxiao
Copy link
Contributor

I think just a "num_spans" or just "num" should be fine. I know we use invert_match elsewhere, but perhaps we could use a different name? Like, operation=min?

I think adding max_spans configuration is enough. Users can set the span_count number they want to sample in the range of min_spans and max_spans. Of course, you can also set only one of them. This is similar to the min_value and max_value of the numeric_attribute policy

Changing the configuration name may not be friendly to users who are using the span_count policy, but adding max_spans will not affect it.

Possible configurations are as follows

# sample traces with minimum 5 spans.
{
    name: test-policy-1,
    type: span_count,
    span_count: {min_spans: 5}
}
# sample traces with maximum 20 spans.
{
    name: test-policy-1,
    type: span_count,
    span_count: {max_spans: 20}
}
# sample traces range of 5 and 20 spans.
{
    name: test-policy-1,
    type: span_count,
    span_count: {min_spans:5, max_spans: 20}
}

@jpkrohling What's your thoughts on this?

@jpkrohling
Copy link
Member

This looks cleaner and users would not be confused about the semantics.

@jiaojialin
Copy link

hello, for this feature, looks very straightforward but useful, could we please help out to get it out asap? Thanks so much!

@yehaotian
Copy link
Author

@mizhexiaoxiao @jpkrohling @TylerHelmuth Thank you all for the work!
Is there a way I can track the release on this? We are currently using opentelemetry-collector-contrib-dev:latest and would like to use the updated policy within a week.

@jpkrohling
Copy link
Member

It will be available on the next release, scheduled for the week of 2023-03-20. If you are building your own distribution, you can use it already based on the main's current state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request processor/tailsampling Tail sampling processor
Projects
None yet
Development

No branches or pull requests

5 participants