-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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/filterprocessor] Ability to filter spans #6341
Conversation
|
|
|
|
|
|
|
|
|
|
|
|
|
1 similar comment
|
CLA pending internal Sony process, should be here shortly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Resurrecting this while waitinf ro CLA |
@mjallday can you remove the stale still waiting on CLA |
Outdated |
I can’t remove the stale message. I’m just an interested neutral party and not affiliated with otel project beyond creating #5838. |
Sorted and ready for your review / merge |
Hey all, My apologies on just leaving this hanging. There were some issues out of my control precluding me from finishing it up. Let me see if I can get this thing updated and fixed with the latest version. |
# Conflicts: # CHANGELOG.md # internal/coreinternal/processor/filterspan/filterspan.go # processor/filterprocessor/factory.go # processor/filterprocessor/factory_test.go
@dmitryax please re-open I have addressed all your feedback and updated this |
Signed-off-by: John Dorman <[email protected]>
@dmitryax is check-links going to block this merge? I didn't touch the links that are broken someone must have removed something from the repo? Nevermind, I was able to track down the right part to link to, it has been updated |
Signed-off-by: John Dorman <[email protected]>
@dmitryax, will you give this another review when you can? |
Any update on this? hoping to use this feature |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor formatting comments but the code looks good otherwise
A few suggestions were missed in the merge of #6341, this PR addresses those.
Signed-off-by: John [email protected]
Description:
Added the ability to use the filterprocessor for spans. The documentation for filter processor currently claims it can do this, however it cannot.
Issue
#5838 ... plus many more
Testing:
Tested locally, all filtering logic is tested by the filterspan internal package, this is just a re-use of that
There are also unit tests that test the filter logic, beyond the filterspan code
Documentation:
Added README.md with a usage example as well as pointing to complete documentation in the attributes processor.