feat(py): Expose validation function for project configs#1860
feat(py): Expose validation function for project configs#1860
Conversation
relay-cabi/include/relay.h
Outdated
| struct RelayStr relay_validate_sampling_configuration(const struct RelayStr *value); | ||
|
|
||
| /** | ||
| * Validate entire project config. If `strict` is true, checks for unknown fields in the input. |
There was a problem hiding this comment.
"checks" in the explanation of strict is confusing. I find more clear the sentence in the relay function itself:
If
strictis true, verify that reserializing the instance results in the same fields.
If you want to have a not-so-defined sentence here to leave room for modifications on the other side, I'd suggest linking to the other function in relay-dynamic-config/src/utils.rs and removing the ambiguity.
There was a problem hiding this comment.
Good suggestion, changed in newest commit.
|
|
||
| [dependencies] | ||
| anyhow = "1.0.66" | ||
| assert-json-diff = "2.0.2" |
There was a problem hiding this comment.
I added this dependency to get more meaningful error messages, i.e. to enumerate any additional fields in the JSON. I used the library we also use for testing in relay-replays. If anyone has better ideas on how to compare JSON, let me know.
| config["foobar"] = True | ||
| with pytest.raises(ValueError) as e: | ||
| sentry_relay.validate_project_config(json.dumps(config), strict=True) | ||
| assert str(e.value) == 'json atom at path ".foobar" is missing from rhs' |
There was a problem hiding this comment.
The error message is a bit cryptic, but at least it contains the name of the problematic field.
relay-cabi/include/relay.h
Outdated
| struct RelayStr relay_validate_sampling_configuration(const struct RelayStr *value); | ||
|
|
||
| /** | ||
| * Validate entire project config. If `strict` is true, checks for unknown fields in the input. |
There was a problem hiding this comment.
Good suggestion, changed in newest commit.
relay-server/src/actors/outcome.rs
Outdated
| use relay_quotas::ReasonCode; | ||
| use relay_quotas::Scoping; | ||
| use relay_system::{Interface, NoResponse}; |
There was a problem hiding this comment.
Please, sort the imports out. Move these to relay stuff
There was a problem hiding this comment.
Thanks, I messed something up during merge. Reverted to master version now.
Some fields that are not being sent by Sentry when they are empty are serialized by Relay. Align with Sentry and do not serialize these fields if they have default values. Reasons: * The main reason is that #1860 introduced a validation function that can be used Sentry-side to check if Sentry accidentally produces any fields that Relay cannot use. This has already surfaced some inconsistencies (see getsentry/sentry#45124), but it requires that Sentry and Relay produce the same fields. * Reduced payload size when sending configs to downstream Relays.
Make sure that the project configs that Sentry generates are consistent with what Relay serializes for downstream usage: * Sampling rules in Relay do not have an `active` field, so stop sending it. * `glob` sampling rules do not have options, so stop sending them. * Sentry unnecessarily sends some empty/default project config fields that Relay can easily derive. Stop sending them. **Why?** * getsentry/relay#1860 introduced a validation function that can be used Sentry-side to check if Sentry accidentally produces any fields that Relay cannot use. This has already surfaced some inconsistencies, but it requires that Sentry and Relay produce the same fields. * Omitting default values reduces the payload size both in redis and on the wire.
Expose a function that validates arbitrary JSON against the
ProjectConfigschema. This can be used in tests on sentry side, or possible even before writing configs to redis.Because librelay releases are lagging behind relay master most of the time, adding new fields might pass validation even though the newest relay would reject them. To prevent this, the function has a
strictmode which checks for stray additional fields.Part of https://github.com/getsentry/team-ingest/issues/60.
#skip-changelog