mirror of
https://github.com/hanzo-fi/webhooks.git
synced 2026-10-30 20:32:55 +00:00
Maxence MaireauxandGitHub
c04477b005
* docs: deep-dive webhooks architecture, prod retry-storm analysis and improvement plan
* fix(worker): treat permanent 4xx as terminal and bound response reads
Non-2xx responses were all retried until the abort-after window elapsed,
so a 403 (bad credentials), 404 (gone) or 405 (wrong method) was retried
for days — the root cause of the production retry storms (Paynovate 403,
ngrok 404, example.com 405).
MakeAttempt now marks 4xx (except 408/429) as 'failed' immediately, honors
an explicit Retry-After on transient responses, and bounds the response
body read to 64 KiB to protect worker memory.
* feat(worker): cap delivery attempts and lower default abort-after
abort-after (default 30d) was the only bound on retries. Combined with the
1m->1h backoff, a dead endpoint was retried ~724 times over 30 days (matching
the observed Tenora retry count) before being marked failed.
Add a --max-attempts hard cap (default 15) enforced by the exponential policy,
and lower the default --abort-after from 30d to 72h. Together they bound a dead
endpoint to a handful of attempts instead of hundreds. The cap is disabled with
--max-attempts=0 for callers that want abort-after-only semantics.
* feat(worker): add attempts retention and orphan reclamation
The attempts table had no retention: success rows were kept forever and
attempts of deleted configs looped in the retry queue indefinitely. In
production this grew the table past 100 GB and dominated cross-AZ RDS cost
(a one-off cleanup cut replication traffic 145GB->0.3GB/15min, ~$39k/yr).
Add a background Retention runner (worker only) that periodically:
- marks pending attempts of deleted configs as 'failed' (FailOrphanedAttempts)
- purges 'success'/'failed' attempts past their retention, in bounded batches
Configurable via --retention-period (1h), --retention-success-delay (30d) and
--retention-failed-delay (90d); a 0 delay disables that purge.
* feat: add OpenTelemetry metrics for webhook delivery
The service emitted only traces and logs, so retry storms were detected from
the AWS bill rather than a dashboard. Add OTLP metrics, wired into both the
serve and worker commands:
- webhooks_delivery_attempts_total{status,status_class} counter
- webhooks_delivery_duration_seconds histogram
- webhooks_retry_queue_depth observable gauge (the leading storm indicator)
Delivery attributes are kept low-cardinality; per-endpoint breakdown stays in
the otelhttp delivery spans to avoid a label explosion across customer endpoints.
* fix: harden retention and Retry-After handling (self-review)
Self-review findings before opening the PR:
- NewRetention: guard against a zero/negative period (time.NewTicker panics
on <= 0 if --retention-period=0 is passed explicitly)
- FailOrphanedAttempts: update in bounded batches — the production orphan
backlog reached millions of rows and a single UPDATE would hold locks and
produce one giant transaction
- Purge/orphan batch subqueries: FOR UPDATE SKIP LOCKED so concurrent
retention runners (multiple worker replicas) do not contend on the same rows
- parseRetryAfter: clamp endpoint-controlled Retry-After to 6h so an absurd
value cannot park a delivery years in the future
* fix(ci): adapt e2e retry spec to terminal 4xx and harden metrics lifecycle
The e2e 'Retries' spec drove its retry scenario with a 404, which the
terminal-4xx change now fails immediately — the spec timed out waiting for
'to retry' attempts. Switch the retry scenario to a transient 500 and add a
dedicated spec asserting that a permanent 4xx fails immediately without ever
entering the retry queue.
Also addresses review findings on the queue-depth gauge:
- only register the DB-backed gauge when a metrics exporter (or in-memory
metrics) is configured — the otlpmetrics module installs a periodic reader
even with the no-op exporter, which would run the COUNT every collect
interval for a value nobody reads
- stop closing the store from the worker lifecycle hook (bunconnect owns the
DB) and register otlpmetrics after postgres in serve, so the final metrics
flush no longer hits a closed database
- add direct Postgres tests for PurgeFinishedAttempts (cutoffs, disabled
retention, batch draining) and FailOrphanedAttempts (orphan reclamation,
live-config preservation, terminal rows untouched)
* fix(webhooks): harden retry retention safeguards
* test(webhooks): cover retry delivery after endpoint recovery
* fix(webhooks): bound retry-after by retry window
* fix(webhooks): address retry review feedback
* fix(webhooks): close retry window loopholes
* fix(webhooks): address retry review followups
* test(webhooks): cover retry store failure paths
* fix(webhooks): keep retry history during retention
76 lines
2.3 KiB
Go
76 lines
2.3 KiB
Go
// Package metrics provides the OpenTelemetry instruments for webhook delivery.
|
|
//
|
|
// Instruments are created lazily from the global meter provider so they bind to
|
|
// the real provider installed by the otlpmetrics fx module at startup (and to a
|
|
// no-op provider, harmlessly, when metrics are disabled — e.g. in unit tests).
|
|
package metrics
|
|
|
|
import (
|
|
"context"
|
|
"sync"
|
|
"time"
|
|
|
|
"go.opentelemetry.io/otel"
|
|
"go.opentelemetry.io/otel/attribute"
|
|
"go.opentelemetry.io/otel/metric"
|
|
)
|
|
|
|
const meterName = "webhooks"
|
|
|
|
var (
|
|
once sync.Once
|
|
deliveryCounter metric.Int64Counter
|
|
deliveryDuration metric.Float64Histogram
|
|
)
|
|
|
|
func instruments() (metric.Int64Counter, metric.Float64Histogram) {
|
|
once.Do(func() {
|
|
meter := otel.GetMeterProvider().Meter(meterName)
|
|
deliveryCounter, _ = meter.Int64Counter(
|
|
"webhooks_delivery_attempts_total",
|
|
metric.WithDescription("Total webhook delivery attempts, by outcome status and HTTP status class"),
|
|
)
|
|
deliveryDuration, _ = meter.Float64Histogram(
|
|
"webhooks_delivery_duration_seconds",
|
|
metric.WithDescription("Duration of the outbound webhook HTTP call"),
|
|
metric.WithUnit("s"),
|
|
)
|
|
})
|
|
return deliveryCounter, deliveryDuration
|
|
}
|
|
|
|
// RecordDelivery records the outcome of a single delivery attempt.
|
|
//
|
|
// Attributes are deliberately low-cardinality (outcome status + HTTP status
|
|
// class). Per-endpoint breakdown is available from the otelhttp delivery spans;
|
|
// we keep it out of metrics to avoid a label-cardinality explosion across the
|
|
// full set of customer endpoints.
|
|
func RecordDelivery(ctx context.Context, status string, statusCode int, elapsed time.Duration) {
|
|
counter, histogram := instruments()
|
|
attrs := metric.WithAttributes(
|
|
attribute.String("status", status),
|
|
attribute.String("status_class", statusClass(statusCode)),
|
|
)
|
|
counter.Add(ctx, 1, attrs)
|
|
histogram.Record(ctx, elapsed.Seconds(), attrs)
|
|
}
|
|
|
|
// statusClass buckets an HTTP status code into a low-cardinality class. A code
|
|
// of 0 means the request never got a response (transport/timeout failure).
|
|
func statusClass(statusCode int) string {
|
|
switch {
|
|
case statusCode == 0:
|
|
return "error"
|
|
case statusCode >= 200 && statusCode < 300:
|
|
return "2xx"
|
|
case statusCode >= 300 && statusCode < 400:
|
|
return "3xx"
|
|
case statusCode >= 400 && statusCode < 500:
|
|
return "4xx"
|
|
case statusCode >= 500:
|
|
return "5xx"
|
|
default:
|
|
return "unknown"
|
|
}
|
|
}
|