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

feat(cascadingfilter): refactor the code, better support of decision history #654

Merged
merged 2 commits into from
Jul 12, 2022

Conversation

pmm-sumo
Copy link
Contributor

@pmm-sumo pmm-sumo commented Jul 6, 2022

This change introduces several new things

  1. Decision history is now stored in LRU cache. This allows for better managing of long-running spans. Also, the decision history is no longer tied to batches and those can be released earlier. The param is controlled by history_size (by default equal to num_traces)

  2. Released traces now have info about which rule sampled them, if possible, e.g.

     -> sampling.rule: STRING(filtered)
     -> sampling.filter: STRING(foo)
  1. The volume of traces that arrived late (after decision_wait) can be now controlled via prior_spans_rate param (by default 50% of spans_per_second). The limit is not counted towards spans_per_second. Those are also marked as such (though no probability is calculated for them). E.g.
     -> sampling.rule: STRING(filtered)
     -> sampling.late_arrival: BOOL(true)
     -> sampling.filter: STRING(probabilistic)
  1. Additional metrics provide information on number of spans filtered, e.g.
otelcol_count_decided_spans{cascading_filter_decision="Sampled",processor="cascading_filter",service_instance_id="f5ab1029-a135-42fc-a70e-7a1931615d8c",service_version="v0.54.0-sumo-0-4-gdde2fd6a89"} 7
otelcol_count_late_spans{cascading_filter_decision="Sampled",processor="cascading_filter",service_instance_id="f5ab1029-a135-42fc-a70e-7a1931615d8c",service_version="v0.54.0-sumo-0-4-gdde2fd6a89"} 2

@github-actions github-actions bot added the go label Jul 6, 2022
@pmm-sumo pmm-sumo force-pushed the cascadingfilter/refactoring branch 2 times, most recently from 6d276ed to 29b9cb5 Compare July 8, 2022 12:08
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Jul 8, 2022
@pmm-sumo pmm-sumo force-pushed the cascadingfilter/refactoring branch 4 times, most recently from dde2fd6 to 0137625 Compare July 8, 2022 12:23
@pmm-sumo pmm-sumo marked this pull request as ready for review July 8, 2022 12:32
@pmm-sumo pmm-sumo requested a review from a team as a code owner July 8, 2022 12:32
@pmm-sumo pmm-sumo force-pushed the cascadingfilter/refactoring branch 2 times, most recently from a893b16 to 92b97d2 Compare July 8, 2022 12:40
@pmm-sumo pmm-sumo changed the title feat(cascadingfiler): refactor the code, better support history feat(cascadingfiler): refactor the code, better support of decision history Jul 8, 2022
@pmm-sumo pmm-sumo force-pushed the cascadingfilter/refactoring branch from 92b97d2 to 81ef121 Compare July 8, 2022 12:44
@andrzej-stencel andrzej-stencel changed the title feat(cascadingfiler): refactor the code, better support of decision history feat(cascadingfilter): refactor the code, better support of decision history Jul 12, 2022
- feat(cascadingfilter): use limit for maximum volume of passed spans for which decisions were made earlier [#654]
- feat(cascadingfilter): store information on which policy filtered the trace in `sampling.filter` [#654]
- feat(cascadingfilter): store information about late span arrival in `sampling.late_arrival: true` [#654]
- feat(cascadingfilter): add `otelcol_count_late_spans` and `otelcol_count_decided_spans` metrics [#654]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I like the names of these metrics:

  • why isn't count a suffix of the metric name?
  • do we even need the count in the name? Wouldn't otelcol_late_spans be enough?
  • Shouldn't metric names be scoped to the component that emits them? So this metric should actually be named otelcol_processor_cacadingfilter_count_late_spans or similar?

@andrzej-stencel andrzej-stencel merged commit 895105d into SumoLogic:main Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation go
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants