Repository navigation
feat!: trace every delivery and record bounded delivery metrics, on forge v1.12.9 and grove v1.8.0 - #21
Merged
Conversation
Forge v1.12.9 traces every request and installs the global tracer provider, and grove v1.8.0 records a db.client span and an operation duration for every query. You get both for free once relay is on these versions, so the spans and metrics that follow only cover work neither of them can see. go-utils moves to v1.4.1 because forge requires it. Every grove line moves together, as AGENTS.md asks.
…ed labels
Relay's metrics had drifted from what they claimed. relay_dlq_size only
ever went up, so it was a counter dressed as a gauge.
relay_deliveries_total mixed an unlabelled series with its labelled
children. And the latency histogram couldn't tell a fast 200 from a
timed-out connection.
You now get these, every series created up front at zero, each label a
fixed set:
- relay_deliveries_attempted_total{outcome}: delivered, retry, dlq or
endpoint_disabled, the same words the attempt history stores.
- relay_deliveries_skipped_total{reason}: rate_limited, orphaned (the
endpoint or event is gone) or store_error.
- relay_delivery_duration_seconds{status_class,error_type}: 2xx to 5xx
when a response came back, otherwise timeout, canceled, dns, tls,
connection or other.
- relay_dlq_entries_pushed_total{result}: ok or error. An error here is
a delivery that failed for good and left nothing to replay.
- relay_deliveries_in_flight: set from the engine's exact count under a
lock, so it can't drift.
relay_events_sent_total and relay_pending_deliveries are unchanged.
relay_deliveries_total, relay_delivery_latency_seconds and relay_dlq_size
are gone, along with the observability fields and RecordDelivery that fed
them. Nothing outside this repo used them.
No label ever carries an ID, tenant, event type, URL, host or error text.
Tests use a small factory that keys series by name and labels the way
Forge's registry does, plus a real Forge app scrape for the forge_ names.
… off it Every delivery attempt now gets a "deliver webhook" span, a root of its own trace, with the HTTP request under it as a client span. The receiver gets a traceparent for that trace. Send and SendReliable get "send event" and "accept event" spans under whatever span you call them with. The client span comes from otelhttp, set up the way forge.InstrumentTransport sets it up, but otelhttp copies the full URL onto the span as url.full and the host as server.address. A webhook URL is customer data, and Slack or Discord style hooks carry their secret in the path. So the tracing layer only ever sees a request addressed to https://REDACTED, and the real URL goes back on underneath it. Transport errors are masked the same way on their way up through otelhttp (a dial error names the host), then restored, so the delivery record still holds the full message for the tenant. We use otelhttp directly rather than forge.InstrumentTransport to keep forge out of the core packages that herald, dispatch and authsome import. Forge v1.12.9 hands Start a context carrying the "start relay" boot span. The engine derived its long-lived context from it, so every delivery and every grove query in every poll would have joined the boot trace. Start now drops the span and the baggage from that context and keeps the rest. An empty poll starts no trace at all. Span statuses name a class (a status code, timeout, connection, a sentinel's name, a Go type), never err.Error(), and exception events carry only exception.type. A missing endpoint or event, a rate-limited delivery and an idempotent duplicate are not errors. The old relay.delivery span is gone with its relay.error attribute, which held raw error text. WithTracer and observability.Tracer still compile but do nothing: spans go to the global provider forge.New installs, so there is nothing to pass.
Replay and ReplayBulk now run under a "replay dlq" span with the mode (single or bulk) and how many entries were re-enqueued. Losing the claim to another caller and replaying an entry that doesn't exist are normal outcomes, so neither fails the span. Anything else does, by Go type. ErrDLQNotFound moves to internal/errs, aliased from the root as the other shared sentinels are, so the dlq package can recognise it. The value and its message are unchanged.
The extension never passed relay.WithLogger, so relay logged to a no-op logger inside a Forge app. A failed DLQ write, a failed delivery update or a dequeue error went nowhere, and the new spans say only that a store call failed and point you at the log. Relay now gets the extension's logger first, so a logger you pass with WithRelayOption still wins.
The observability page now lists every span with its kind, parent and attributes, and every metric under the name Forge scrapes it as, with each label's full set of values and a few queries to start from. It says what the receiver sees (traceparent, never your baggage), why the client span reads https://REDACTED, and which metrics and options went away. The README, AGENTS.md, the configuration and Forge guides, the architecture table and the Go package reference stop offering WithTracer and NewTracer and stop describing a latency histogram that no longer exists.
Grove v1.8.0 starts a db.client span under any valid span in a query's context, even one that ended long ago. Relay's engine and wake listener are started inside Forge's "start relay" span, so if they kept it, every idle poll would add a query to the boot trace for the life of the process. Start already strips the span. These tests hold it there: a store wrapper records whether the poll, the pending count and the wake listener see a valid span while relay runs under one (they must not), and that a delivery's own queries run under its "deliver webhook" span. Both fail if you take the stripping out.
WithHTTPClient copies your client and wraps its transport in the same tracing relay uses for its own: a client span with the URL redacted and the trace context on the request. Its timeout, redirect policy and jar carry over. The README, the configuration and delivery pages and the observability page now say so, and warn you off passing a transport that's traced already, because that tracing would sit underneath and see the real URL.
… that can't collide A redirect sends a second request to whatever the Location says, and the endpoint controls that. The new test redirects to a URL with its own secret and checks each hop gets a client span with url.full redacted and that neither host, path nor secret reaches a span. The truncated-body test now checks the client span's status reads only "connection" and runs the same leak check. The root tests used tenant "t1" as a leak needle. A TypeID's base32 suffix contains "t1" now and then, so the test failed about one run in fifty. Needles are now strings no ID can produce by chance, such as tenant-acme and alice@example.com, and the acceptance test sets up its own tenant and app to match.
"delivery failed permanently" logged result.Error. For a transport failure that is err.Error() from the HTTP client, which quotes the full endpoint URL (path secret, query token and all) and the address it dialled. While relay logged to a no-op logger nobody saw it. Now that the extension passes the app's logger, it would have put customer credentials in your logs and undone the span redaction. The log line now carries the attempt, the status and the same failure class the span uses (a status code, or timeout, connection and so on). "retry scheduled" gets the class too. The full text stays on the delivery record, where its tenant can read it. We went through every log call the logger fix brought to life. The rest log IDs, counts, statuses and store errors. Store errors are relay's own database messages, and none of relay's writes put an endpoint URL in an error. A new test runs deliveries that succeed, retry, get rejected, are refused, are redirected to a secret URL and hit failing store writes through a capturing logger, and checks that the secret, the token, the host and the address never show up in a message or a field.
With no transport of its own, the delivery client used to read http.DefaultTransport each time it sent, as any plain client does. The tracing wrapper resolved it once, when relay was built, so a transport swapped in later (httpmock, gock, an app's own default) was ignored. The wrapper now resolves a nil transport per request again.
The upgrade notes named the retired series but not the code behind them. observability.Metrics lost RecordDelivery and the DeliveriesTotal, DeliveryLatency and DLQSize fields, so code that used them won't compile. The page now says so and names what replaced them.
juicycleff
changed the title
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.
In a Forge app you get a trace per delivery attempt and real delivery metrics with nothing to configure. Send a webhook and you'll see a
deliver webhooktrace with anHTTP POSTspan under it, and the receiver gets itstraceparent. Theforge_relay_*series are on/_/metricsfrom the start.Webhook URLs stay out of spans and logs. Slack style hooks keep their secret in the path, so otelhttp only ever sees
https://REDACTEDand the real URL goes back on underneath. Errors on spans and in logs carry a class (503,timeout,connection); the full text stays on the delivery record.Spans
send eventrelay.event.type,relay.event.id,relay.fanout.count,relay.event.duplicateaccept eventrelay.event.type,relay.event.id,relay.fanout.count,relay.acceptance.createddeliver webhookrelay.delivery.id,relay.event.id,relay.endpoint.id,relay.event.type,relay.delivery.attempt,relay.delivery.outcome,http.response.status_code,error.typeHTTP POSTdeliver webhookurl.fullandserver.addressredactedreplay dlqrelay.dlq.mode,relay.dlq.replayed,relay.dlq.entry.id,relay.dlq.already_replayedMetrics
forge_relay_events_sent_totalforge_relay_deliveries_attempted_totaloutcome: delivered, retry, dlq, endpoint_disabledforge_relay_deliveries_skipped_totalreason: rate_limited, orphaned, store_errorforge_relay_delivery_duration_secondsstatus_class: 2xx, 3xx, 4xx, 5xx, other, none;error_type: none, timeout, canceled, dns, tls, connection, otherforge_relay_dlq_entries_pushed_totalresult: ok, errorforge_relay_pending_deliveriesforge_relay_deliveries_in_flightBreaking
The series
relay_deliveries_total,relay_delivery_latency_secondsandrelay_dlq_size(which only ever went up) are gone, so move alerts first. So is the Go API behind them:observability.Metrics.RecordDeliveryand theDeliveriesTotal,DeliveryLatencyandDLQSizefields.WithTracerandNewTracerstill compile and do nothing.Other changes
relay.WithHTTPClientcommits from local main. A supplied client keeps its transport, timeout, redirect policy and jar, and gets the same tracing.start relayboot span, or every idle poll would add a grove span to the boot trace.Risks
Nothing stops you passing
WithHTTPClienta client that's already traced, likeforge.HTTPClient(). Its tracing sits under relay's and records the real URL on its own span. The docs warn against it.Release
Once CI is green on main:
Herald, dispatch and authsome re-pin afterwards.
Testing
go test -race -count=1 ./...passes with the Postgres, Mongo and Redis containers. Tests push secret URLs through deliveries, redirects and truncated bodies, and nothing reaches a span or a captured log. Lint, gosec, govulncheck and docs checks are clean.