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(filter): Add sampling based on transaction name #1058

Merged
merged 6 commits into from
Aug 24, 2021

Conversation

RaduW
Copy link
Contributor

@RaduW RaduW commented Aug 19, 2021

Adds the ability to sample events & transactions based on the transaction name.
This is supported for both event content and trace content.
This PR introduces another optional field in the trace context (transaction_name).

INGEST-266, INGEST-267, INGEST-268

@RaduW RaduW requested a review from a team August 19, 2021 13:04
@@ -365,6 +365,10 @@ impl FieldValueProvider for Event {
Value::Bool(relay_filter::browser_extensions::matches(self))
}
"event.web_crawlers" => Value::Bool(relay_filter::web_crawlers::matches(self)),
"event.transaction" => match self.transaction.0 {
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you are doing those conversions, you can use the methods provided by the ToValue trait for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IntoValue converts into our relay_general::Value type , this is a serde_json::Value ... are you suggesting using the realy_geneeral::Value type ?

Copy link
Member

Choose a reason for hiding this comment

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

IntoValue::??? and then Into::into to get from our Value to serde's Value... duked out IRL, your way is more readable albeit verbose

@RaduW RaduW merged commit 68bb376 into master Aug 24, 2021
@RaduW RaduW deleted the feat/sampling-on-transaction-name branch August 24, 2021 14:46
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