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.

Sign up for GitHub

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

Jump to bottom

Add filters support to generic forwarding #2742

Merged
merged 8 commits into from
Aug 4, 2023
Merged

Add filters support to generic forwarding #2742

merged 8 commits into from
Aug 4, 2023

Conversation

Blinkuu
Copy link
Contributor

@Blinkuu Blinkuu commented Aug 1, 2023 •
edited
Loading

What this PR does:

This PR adds support for filtering spans and span events in the Generic Forwarding feature. The filtering feature is needed for k6 x Tempo GA.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@Blinkuu Blinkuu added the enhancement New feature or request label Aug 1, 2023
@Blinkuu Blinkuu self-assigned this Aug 1, 2023
@Blinkuu Blinkuu force-pushed the add_filters_support_to_generic_forwarding branch 2 times, most recently from a3d63c8 to 908ddcd Compare August 1, 2023 14:35
@zalegrala
Copy link
Contributor

I'm wondering if pkg/spanfilter can help here.

@joe-elliott joe-elliott added operations enhancement New feature or request and removed enhancement New feature or request operations labels Aug 2, 2023
@Blinkuu Blinkuu force-pushed the add_filters_support_to_generic_forwarding branch from 908ddcd to 6083889 Compare August 2, 2023 12:47
@Blinkuu
Copy link
Contributor Author

Blinkuu commented Aug 2, 2023

I'm wondering if pkg/spanfilter can help here.

The pkg/spanfilter does not allow filtering on trace_id string, which we require.

@Blinkuu Blinkuu force-pushed the add_filters_support_to_generic_forwarding branch from 6083889 to 6eb7102 Compare August 2, 2023 12:51
@Blinkuu Blinkuu marked this pull request as ready for review August 2, 2023 14:00
@Blinkuu Blinkuu added the Urgent label Aug 3, 2023
knylander-grafana
knylander-grafana reviewed Aug 3, 2023
View reviewed changes
Copy link
Contributor

@knylander-grafana knylander-grafana left a comment

Choose a reason for hiding this comment

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

Thank you for adding doc! The doc updates look good.

yvrhdn
yvrhdn approved these changes Aug 3, 2023
View reviewed changes
Copy link
Member

@yvrhdn yvrhdn left a comment

Choose a reason for hiding this comment

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

This looks really good! Just left a little nit about the changelog

CHANGELOG.md Outdated Show resolved Hide resolved
@yvrhdn yvrhdn merged commit 72118ab into main Aug 4, 2023
@yvrhdn yvrhdn deleted the add_filters_support_to_generic_forwarding branch August 4, 2023 14:54
@knylander-grafana knylander-grafana mentioned this pull request Aug 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@knylander-grafana knylander-grafana knylander-grafana left review comments

@yvrhdn yvrhdn yvrhdn approved these changes

@joe-elliott joe-elliott Awaiting requested review from joe-elliott joe-elliott is a code owner

@annanay25 annanay25 Awaiting requested review from annanay25

@mdisibio mdisibio Awaiting requested review from mdisibio mdisibio is a code owner

@mapno mapno Awaiting requested review from mapno mapno is a code owner

@zalegrala zalegrala Awaiting requested review from zalegrala zalegrala is a code owner

@electron0zero electron0zero Awaiting requested review from electron0zero electron0zero is a code owner

@ie-pham ie-pham Awaiting requested review from ie-pham ie-pham is a code owner

@stoewer stoewer Awaiting requested review from stoewer stoewer is a code owner

Assignees

@Blinkuu Blinkuu

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Blinkuu @zalegrala @yvrhdn @knylander-grafana @joe-elliott