Repository navigation
[pull] main from hookdeck:main - #190
Merged
Merged
Conversation
…metrics/attempts (#1079) * feat(destregistry): record per-attempt latency on the attempt Time the provider's Publish call in PublishEvent and stamp the elapsed milliseconds on the attempt as LatencyMs, exposed as latency_ms on the attempt object. Measuring at the registry rather than inside a provider covers every destination type in one place, and it is the same call that DeliveryTimeout already bounds, so a timed-out attempt records roughly the timeout rather than nothing. Successful and failed attempts both get a value; an attempt that never reached the provider (publisher resolution or validation failure) has none, and the field is nullable so that stays distinguishable from a fast reply. * feat(logstore): persist attempt latency in ClickHouse, Postgres and memory Add a nullable latency_ms column to the attempts table in both SQL stores (ClickHouse 000004, Postgres 000011) and carry the field through insert, list and retrieve in all three drivers. No index: the column is only ever aggregated, never filtered on. Nullable on purpose. Attempts written before this migration have no value, and NULL is skipped by AVG / percentile aggregates, so historical rows do not drag the numbers down and the API can report them honestly as null. The CRUD conformance test now asserts the value round-trips and that nil comes back as nil. * feat(metrics): latency measures on GET /metrics/attempts Add avg_latency, p50_latency, p95_latency and p99_latency (milliseconds) to the attempt metrics API so response time can be sliced and bucketed with the same dimensions, filters and granularity as the existing measures. ClickHouse computes the three percentiles from a single quantilesTDigest state per group (bounded memory, one pass; quantileExact would materialize every value per group). Postgres uses percentile_cont, the in-memory driver linear interpolation between closest ranks, i.e. the same definition. The conformance dataset pins avg exactly and the percentiles within a tolerance that still resolves to a single expected value, so the t-digest estimate is checked, not just tolerated. Latency has no meaningful zero, so unlike counts and rates these measures are null rather than 0 for a bucket in which no attempt recorded a latency, including empty zero-filled buckets. A 0ms point on a response time chart would be a lie. * chore(bench): seed attempt latency and add latency metrics cases Seed latency_ms in both backends (20..520ms body, every 200th event's attempts at the 5000ms timeout as a p99 tail) and add avg / percentile cases, including the per-destination hourly and single-destination daily shapes a dashboard issues. Also fix the event seeds, which still inserted events.destination_id: that column was replaced by matched_destination_ids in migrations 000002 (CH) and 000007 (PG), so the scripts no longer ran on main. * refactor(chlogstore): use quantilesTiming for latency percentiles Purpose-built for millisecond latencies: deterministic, exact below 1024ms and within 16ms up to 30s, one bounded state per group. Benchmarks ~15% faster than quantilesTDigest on the same grouped month query. * test(destregistry): drop the latency upper bound The lower bound already proves the publish call is timed; a 500ms ceiling can fail on a busy parallel CI runner. * docs: latency_ms in the operator event attempt example Operator event payloads embed *models.Attempt, so the field appears there too. * chore(bench): paired dashboard cases with and without latency The marginal cost of avg + p95 latency on count + error_rate by destination, hourly over a day and a month and daily over a month.
) * refactor: move proxy chain dialer to internal/proxychain The CONNECT chain dialer and proxy URL parsing move out of destregistry into their own package so non-HTTP clients can tunnel through the same chain. Renames: ParseProxyURL -> proxychain.Parse, RedactedProxyURL -> proxychain.Redact, ProxyConnectError -> proxychain.ConnectError. NewDialer's DialContext tunnels to any addr through every hop; the HTTP client keeps passing all but the last hop. The in-process CONNECT proxy test helper moves to proxychaintest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(publishmq): PUBLISH_PROXY_URL for the RabbitMQ publish queue Tunnels the RabbitMQ publish queue connection through an HTTP CONNECT proxy chain (same format as DESTINATIONS_WEBHOOK_PROXY_URL), so a broker that allowlists source IPs sees the proxy's address. amqp091 layers amqps TLS over the tunneled conn, so broker TLS stays end to end. The dial keeps amqp091's defaults (10s heartbeat, en_US locale) and honours the URI's connection_timeout. Other publish providers connect directly: the variable is ignored with one info log. Internal delivery/log queues never use it. A malformed value fails startup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: PUBLISH_PROXY_URL Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(publishmq): rely on amqp091 default locale, build publish queue config once Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
) * fix(mqs): resubscribe RabbitMQ subscription after connection loss A RabbitMQ subscription was bound to the connection it was opened on. Once that connection closed (broker restart, closed at the broker, proxy restart), every Receive failed until the consumer hit max consecutive errors and the worker stopped. Receive now redials and reopens the subscription when its connection is closed, then returns the original error so consumer backoff still applies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(mqs): don't redial RabbitMQ after queue cleanup Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )