ref: Buffer envelopes for broken project states #1856
Conversation
There was a problem hiding this comment.
@jan-auer what is the reason we treat invalid states different than missing states? To me it seems that in both cases, we want to buffer events and retry to fetch the config. So maybe we can get rid of the concept of an invalid state altogether (except maybe for logging purposes)?
There was a problem hiding this comment.
Reviewed again together with @jjbayer:
- In
update_state, we never re-trigger fetching of states if there's an invalid state. This is now a problem, because we want to keep envelopes queued. This will require a change toupdate_state. - In
enqueue_validation, we should skip states that are invalid - You change in
check_envelopecovers the fast-path. Potentially we might want to catch this inhandle_check_envelopethough?
We explicitly take the risk of overflowing buffers now, which will be addressed through persistent queueing.
relay-server/src/actors/project.rs
Outdated
| // Intentionally ignore all errors. Fallback sampling behavior applies in this case. | ||
| if let Some(state) = self.valid_state() { | ||
| if self.valid_state().is_some() { |
There was a problem hiding this comment.
This check is redundant now, since get_cached_state already checks for expiry. We could document on flush_sampling that this should not be called with an expired state.
Thinking through this again, I suggested a wrong change earlier. As the comment above states, we want to skip invalid and missing project states for this assignment. This is important because the public key from the DSC can point to a project that does not exist or is unavailable in the same Sentry instance. In this case, fallback sampling behavior must apply.
* master: feat(metrics): Tag the sample decision on count_per_root_project (#1870) feat(protocol): Add sourcemap debug image type to protocol (#1869) ref(statsd): Revert back the adition of metric names as tag on Sentry errors (#1873) feat(profiling): Add PHP support (#1871) fix(panic): revert sentry-types to 0.20.1 (#1872) ref(server): Use async/await in all endpoints (#1862) ref: Buffer envelopes for broken project states (#1856) meta: Remove accidentally added GeoIP file (#1866)
Check request only for the valid project configs, otherwise we reject incoming events immediately with
invalid data (project_state), since there is nothing to check against.With this change we will still keep buffering even if the resolved project state is invalid. After the state is updated, or pending envelopes will be processed again.
#skip-changelog
fix: #1787