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(cascadingprocessor): Added status code filtering to cascading filter processor #1600

Merged
merged 13 commits into from
Jun 13, 2024

Conversation

chan-tim-sumo
Copy link
Contributor

@chan-tim-sumo chan-tim-sumo commented Jun 7, 2024

Links:

  1. github issue: cascadingfilterprocessor should support filtering on trace status #1573
  2. jira: https://sumologic.atlassian.net/browse/OSC-726
  3. cascading filter processor

From the github issue, they wanted to be able to filter traces by status code for ROOT span only. Currently the only way to filter is to use minimum number of errors which basically checks the minimum number of errors within a trace, which isn't as great.

For example, if minimum number of errors is set to 1 (this means 1 or more errors):
Trace 1:

  • span1 (root) = OK
  • span2 = ERROR
  • span3 = OK

Trace 2:

  • span1 (root) = ERROR
  • span2 = ERROR
  • span3 = OK

This would return trace 1 and 2. But what they would want is to accept trace 2 only, with status code filtering, using the same example from above, will return trace 2 only.

@chan-tim-sumo chan-tim-sumo requested review from a team as code owners June 7, 2024 19:33
@chan-tim-sumo chan-tim-sumo removed the request for review from a team June 7, 2024 19:43
sumo-drosiek
sumo-drosiek previously approved these changes Jun 11, 2024
Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

Just one more question. Is status_code case sensitive? Is it ok to type error, Error, eRRoR? If not, can we fix the user's input or raise an error?

@sumo-drosiek sumo-drosiek self-requested a review June 11, 2024 05:14
@sumo-drosiek sumo-drosiek dismissed their stale review June 11, 2024 05:14

wanted to comment 😓

@chan-tim-sumo
Copy link
Contributor Author

chan-tim-sumo commented Jun 11, 2024

Just one more question. Is status_code case sensitive? Is it ok to type error, Error, eRRoR? If not, can we fix the user's input or raise an error?

good point, added! Mentioned in the Readme that the values are "Error" "Ok" or "Unset" but can't completely rely on that :D

Output after throwing an error if a user uses status code as "ERROR" instead of "Error":

Error: failed to build pipelines: failed to create "cascading_filter" processor, in pipeline "traces/default": Invalid status code: must be one of 'Error', 'Ok', or 'Unset' 
2024/06/11 10:07:33 collector server run finished with error: failed to build pipelines: failed to create "cascading_filter" processor, in pipeline "traces/default": Invalid status code: must be one of 'Error', 'Ok', or 'Unset' 

link ref: https://github.com/open-telemetry/opentelemetry-collector/blob/main/pdata/ptrace/status_code_test.go#L12

Copy link
Contributor

@sumo-drosiek sumo-drosiek left a comment

Choose a reason for hiding this comment

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

LGTM, one minor comment

@chan-tim-sumo chan-tim-sumo enabled auto-merge (squash) June 13, 2024 15:53
@chan-tim-sumo chan-tim-sumo merged commit 76db872 into main Jun 13, 2024
24 checks passed
@chan-tim-sumo chan-tim-sumo deleted the chan-tim_statusCodeFiltering branch June 13, 2024 15:55
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

Successfully merging this pull request may close these issues.

2 participants