[pull] main from hookdeck:main - #174
Merged
Merged
Conversation
) * perf(retrymq): cut idle Redis polling to one command per interval The retry monitor polled every 100ms, and each poll cost 5 client-observable commands over 2 round trips: a MULTI/HMGET/TIME/EXEC to fetch the queue's vt/delay/maxsize and the server clock, then the EVALSHA that used them. The cost was per monitor instance and independent of traffic, so an empty queue cost as much as a busy one and total load scaled with replica count. Make receiveMessage self-contained — it reads vt from the :Q hash and calls TIME itself, which upstream RSMQ could not do under pre-Redis-5 verbatim script replication. When nothing is due it also returns the time until the zset's earliest score, so the monitor sleeps until the next message comes due instead of a flat interval. The zset holds both not-yet-due retries and in-flight messages hidden by vt, so that score is the correct wake time in either case, and every sleep is still capped. RETRY_POLL_BACKOFF_MS is redefined as that cap, default 100 -> 30000, and the effective value is clamped to the shortest configured retry delay so the idle interval can never make a retry late. The consecutive-error backoff ladder no longer derives from it, keeping the ~1 minute of transient-infra tolerance fixed. At a 30s cap: 129.6M -> 86.4K commands per month, per monitor. Closes #1014 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(scheduler): fix goroutine leak, data race, and tight timing in new tests - IdleSleepWakesOnDueMessage: wait for the Monitor goroutine to exit after cancel so it cannot log via t after the test completes, and assert the execution window as an elapsed range with a looser upper bound (3s) for loaded CI. - MonitorRetriesTransientErrors: guard the msgs slice with a lock (msgLog helper) — exec runs on the monitor's goroutines, so the require.Eventually read raced with appends under -race. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(scheduler): synchronize executor message capture in pre-existing tests The scheduler has always run exec on the monitor's goroutines, so the unsynchronized msgs slices in TestScheduler_Basic, ParallelMonitor, VisibilityTimeout, CustomID, and Cancel raced with test-side reads. These races predate this branch (reproduced on main) but fail the package under -race. Reuse the msgLog helper everywhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(scheduler): wait for monitor goroutines to exit before Shutdown Test cleanups cancelled the monitor context and called Shutdown without waiting for the Monitor goroutine to exit. Shutdown breaks the Redis client mid-poll, so the still-running monitor logs a receive-error Warn through the zaptest logger after the test has finished — flagged by the race detector as a write to testing.T past completion. Add a startMonitor helper that returns a wait function and apply the cancel → wait → Shutdown ordering at every monitor spawn site. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(config): honor explicit retry_poll_backoff_ms; 0 sentinel = auto Capping the configured backoff at the shortest retry delay could silently override an explicit user value downward (retry_schedule [5, ...] + explicit 10000ms → forced to 5s, doubling the intended idle Redis cost). Replace the cap with a sentinel: default 0 means auto — min(30s, shortest configured retry delay), so retries are never late — while an explicit positive value is honored as-is as a fixed maximum idle sleep. Validation now rejects retry_schedule entries < 1, retry_interval_seconds < 1 when no schedule is set, and negative retry_poll_backoff_ms at startup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…#1019) * fix(otel): make per-signal exporter and protocol configurable via env OpenTelemetryConfig has had per-signal Traces/Metrics/Logs sections all along, but the three shared one pair of env tags, so OTEL_EXPORTER and OTEL_PROTOCOL configured all three at once. Enabling OTel for one signal enabled the other two against localhost:4317, where they retried into a dead socket for the life of the process. Resolve the exporter per signal after env parsing: OTEL_{SIGNAL}_EXPORTER first, then OTEL_EXPORTER, then endpoint inference — a per-signal endpoint enables that signal and disables the rest, the generic endpoint enables all three, no endpoint leaves all three on as before. Resolve the protocol per signal from the spec variables that were silently ignored: OTEL_EXPORTER_OTLP_{SIGNAL}_PROTOCOL, then OTEL_EXPORTER_OTLP_PROTOCOL, then OTEL_PROTOCOL, then grpc. This wires up getProtocol, which had no callers and read from a viper instance the config package never constructs. http/protobuf is accepted as an alias for http. Fixes #1015 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(otel): endpoints decide which signals export; validate exporter values Endpoint gating and exporter selection become orthogonal. Endpoints decide which signals are enabled: per-signal endpoints alone enable exactly those signals, the generic endpoint enables all three, no endpoints keeps all three on the SDK default. Exporter variables decide how an enabled signal exports — otlp or console/stdout — and none still disables one; an exporter value can turn a signal off, never on. This replaces the previous rule where an explicit exporter could re-enable a signal that endpoint gating had turned off; wanting a signal on is now spelled by naming its endpoint. Exporter values are validated at startup against otlp/console/stdout/ none, so a typo fails config validation instead of silently disabling the signal. Known edge: a typo on a signal already gated off by endpoints is overwritten to none before validation sees it. The protocol error message and desc tags now name http/protobuf and stdout, and the OpenTelemetry feature doc gains a Signal Configuration section covering endpoints, exporters, protocol precedence, and the process-env-only nature of the spec variables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
) Addresses the CVEs reported in #1012: - CVE-2026-39822 (GO-2026-4970, os.Root symlink escape) — not reachable from outpost, fixed by the toolchain bump regardless. - CVE-2026-42505 (GO-2026-5856, crypto/tls ECH PSK leak) — reachable via every TLS dial, but ECH is never enabled. - CVE-2026-56852 (GO-2026-5970, x/text norm.Iter infinite loop) — reachable through the webhook delivery transport's IDNA handling, pgx, and Kafka SCRAM. The one with real exposure. Also picks up GO-2026-6061 (grpc xDS RBAC / HTTP2 server), which the issue did not list but govulncheck flags as reachable via Pub/Sub Subscribe. govulncheck now reports no vulnerabilities. Everything else moves to latest with `go get -u ./...`, except the OpenTelemetry log packages, which stay pinned at v0.20.0: otel/log v0.21.0 removes log.Value/log.KeyValue, which breaks uptrace/opentelemetry-go-extra/otelzap. That module has been unmaintained since 2024 with no compatible release, so replacing it is its own change. CI Go pins move to 1.26.5, including spec-sdk-tests which was still on 1.23, and the two stragglers still on golang:1.23-alpine.
* fix(test): give the S3 storage-class subtest its own config map
The subtest wrote into the Config map shared by every subtest in
TestAWSS3Destination_Validate, all of which run in parallel, so it could
turn the valid-destination case invalid.
* fix(test): stop a redelivered Kinesis record from derailing the suite
Every test in a provider's PublisherSuite reads one channel fed by one
consumer built in SetupSuite. GetRecords can hand back a record an earlier
call already returned — seen against localstack with no error reported and
the shard iterator advanced normally — and a single redelivered record then
shifts every later read by one, so each test verifies some other test's
event.
The symptoms all trace to that one shift. TestBasicPublish passes and its
record comes around again, so TestClosePublisherDuringConcurrentPublish and
TestConcurrentPublish panic on a payload with no message_id, and
TestPublishWithDeliveryMetadata reports an event-id mismatch. It needs a
slow localstack to show up, which is why it reads as a cold-start flake:
restarting the container right before the run failed 10 of 20 times, and
0 of 20 with this change.
Records within a shard are ordered, so tracking the last sequence number
handed to the suite and dropping anything at or before it makes the stream
deliver-once from the suite's point of view. Kinesis is the only consumer
built on a position token; the other nine ack individually or are pushed,
so none of them can replay this way.
Also report a message that carries no message_id as a plain failure naming
the payload. That check previously panicked inside a type assertion, which
said nothing about what had gone wrong.
* fix(test): isolate Kinesis streams per test, gate on infra readiness
Each test in the Kinesis suite now creates its own stream and consumer
rather than sharing one built in SetupSuite, so nothing a test leaves
behind can reach the next one. Four stream create/deletes add about two
seconds to the suite.
The sequence-number check stays, for its own reason: a stream is an
at-least-once log and the consumer has to be idempotent whatever the
suite does around it. It also covers a record redelivered inside the test
that published it, which per-test streams cannot.
Ensure{RabbitMQ,LocalStack,Kafka,GCP} handed back endpoints supplied
through the environment without checking that anything was listening.
`docker compose up -d` returns once containers are created, not once they
accept connections, so a suite that connects immediately could lose the
race — seen as a RabbitMQ connection reset partway through SetupSuite.
Each now waits up to 30 seconds before the first test runs: an AMQP dial
for RabbitMQ, the health endpoint for LocalStack, a TCP dial for Kafka and
the Pub/Sub emulator. Only on the environment path; testcontainers already
applies its own wait strategies.
* fix(test): make the suite runnable without TESTINFRA
Running the tests without TESTINFRA=1 is supposed to fall back to
testcontainers. It had stopped working: of nine packages that need
infrastructure, six failed before running a single assertion. None of it
was visible day to day, because the failures are all in code the
TESTINFRA=1 path never executes.
Pin the images. The fallback asked for localstack/localstack:latest,
which now resolves to a licensed build that exits during startup, and for
postgres:latest and clickhouse-server:latest, which have drifted past
what the code assumes. Tags live in .env.test, which compose reads via
--env-file and testinfra reads via viper, so the two ways of providing
infrastructure run the same versions instead of drifting apart.
Fix the Kafka container. Its host port was mapped with a syntax
testcontainers rejects as of v0.42, so the package had been unrunnable
across two dependency bumps; bind the port through HostConfigModifier
instead, and take one the OS reports free rather than hardcoding 19092,
which two test binaries would otherwise contend for. The broker also
needs a JAAS file, and needs its controller listener on the address the
image's quorum voters name — without that it starts, fails to register,
and shuts down again with the port already open.
Keep shared containers for the life of the process. They were terminated
when the first suite in a binary finished, by which point the sync.Once
guarding each one had already fired, so every later suite in that binary
connected to an endpoint with nothing behind it. The testcontainers
reaper already removes them at exit.
Gate every Ensure on readiness, whoever started the service. Neither
`compose up -d` returning nor a wait strategy seeing an open port proves
a service will complete a handshake, and both failure modes surface
inside a test rather than at startup. Probes now speak the protocol:
authenticate to Kafka, connect to Postgres, query ClickHouse.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* docs(test): describe the fallback by behaviour, not by package path
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <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.
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.
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 : )