Conversation
WarnThreshold, WarnSweepMaxPerTick, and DisableSweepMaxPerTick were compiled constants. A self-hosted deployment's traffic volume can make WarnThreshold unreachable in either direction, and the two per-tick caps are the only incident-response lever for a real e2a-side outage; turning either down used to need a release. Adds webhook.warn_threshold and webhook.sweep_max_per_tick (env: E2A_WEBHOOK_WARN_THRESHOLD, E2A_WEBHOOK_SWEEP_MAX_PER_TICK), both defaulting to the current compiled values so an unconfigured self-host is unchanged. Config.Validate rejects anything below 1: a threshold or cap of 0 would silently break the feature rather than disable it. Store.SetWebhookHealthLimits carries the override the same optional-setter way SetDKIMCipher and friends do, so NewStore(pool) and every existing test are untouched. AutoDisableThreshold/AutoDisableWindow stay compiled, per the issue's own scope: they interact with the GA-frozen retry envelope and want more thought before being exposed. Fixes tokencanopy#863
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WarnThreshold, WarnSweepMaxPerTick, and DisableSweepMaxPerTick were compiled constants, so a deployment whose traffic can't accumulate the compiled default's failures could never warn, and turning either per-tick cap down during a real outage meant shipping a release. Scoped this to the two items you flagged as worth having first: WarnThreshold and the per-tick caps. AutoDisableThreshold/AutoDisableWindow stay compiled, as you suggested.
New
webhook.warn_thresholdandwebhook.sweep_max_per_tick(env:E2A_WEBHOOK_WARN_THRESHOLD,E2A_WEBHOOK_SWEEP_MAX_PER_TICK), both defaulting to today's compiled values so an unconfigured deployment doesn't change. One knob covers bothWarnSweepMaxPerTickandDisableSweepMaxPerTickrather than two, since your own example only had onesweep_max_per_tickkey and they're both 100 today for the same incident-response reason. Validate rejects anything below 1, matching your validation note.Went under the existing
webhook:block (internal_sink_url's home) rather than a newwebhooks:block: its own doc comment already says "webhook-delivery settings other than the fan-out engine choice," which is exactly this. Store.SetWebhookHealthLimits carries the override the same optional-setter way the rest of Store's config does, so NewStore(pool) and every existing test are unchanged.Added TestSetWebhookHealthLimits_OverridesWarnThreshold and TestSetWebhookHealthLimits_OverridesSweepMaxPerTick (the latter also proves the cap drains across ticks rather than dropping), plus TestWarnFailingWebhooks_DefaultWarnThresholdUnaffectedWithoutSetter for the no-override case, and a config load/env/validate test mirroring the existing trash.retention_days one. Full
go test ./internal/identity/...and./internal/config/...pass,gofmtandgo vetclean.Not done: the config.example.yaml documentation block, since the existing
webhook:key isn't documented there either (it looks deliberately internal-facing). Happy to add one if you want it there for these two operator-facing fields specifically.Fixes #863