Skip to content

feat(metrics): Change rules schema for latest release#41737

Merged
iambriccardo merged 4 commits into
masterfrom
riccardo/feat/TET-563
Nov 28, 2022
Merged

feat(metrics): Change rules schema for latest release#41737
iambriccardo merged 4 commits into
masterfrom
riccardo/feat/TET-563

Conversation

@iambriccardo

Copy link
Copy Markdown
Contributor

This PR aims at changing the rules schema for latest release boosting rules.

The new schema uses the eq operator and the null value instead of glob and not.

# When environment is None, it will be mapped to equivalent null in json.
# When Relay receives a rule with "value": [null] it will match it against events without
# the environment tag set.
"value": [boosted_release.environment],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When the value in the event is null, EqCondition does not go into the Array branch, so I think we need just the value here:

Suggested change
"value": [boosted_release.environment],
"value": boosted_release.environment,

https://github.com/getsentry/relay/blob/a1a12a06f1fdd53566965af4727dc6675268f5b5/relay-sampling/src/lib.rs#L66-L75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is why I wanted an ingest folk 👀, they know their baby well.

@jjbayer jjbayer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can drop the list entirely (see comment). Apart from that, LGTM!

Comment on lines +116 to +118
"value": [boosted_release.environment]
if boosted_release.environment
else None,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"value": [boosted_release.environment]
if boosted_release.environment
else None,
"value": boosted_release.environment

eq also works with a single value, it then goes into this branch:
https://github.com/getsentry/relay/blob/ae0f47aa5d80bf9086f8bef6575c3713eaf428e2/relay-sampling/src/lib.rs#L68

]
assert generate_rules(default_project) == expected
config_str = json.dumps({"rules": generate_rules(default_project)})
validate_sampling_configuration(config_str)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for these!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants