fix(processing): count processed events in the messages-processed metric - #544
Merged
Xhristin3 merged 1 commit intoAug 24, 2026
Conversation
The messagesProcessed counter was never incremented outside its own unit test: StreamSession.pump() emitted the "processed" event after a successful publish but no production path touched the metrics module, so xstreamroll_messages_processed_total stayed at zero and the processed-vs-error ratio was meaningless. Increment the counter in pump() immediately after publish resolves — after any retries, and never for dead-lettered events — so the metric reflects exactly the events the worker successfully delivered.
Closed
4 tasks
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
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.
Summary
Closes #521
xstreamroll_messages_processed_totalwas always zero:incrementProcessed()had no production caller. The worker's per-event success path (StreamSession.pump()inxstreamroll-processing/src/session.ts) emitted a"processed"event after a successful publish but never touched the metrics module, so processed-vs-failed counts were unobservable. This change increments the counter inpump()immediately afterpublish()resolves — after any retries, and never for dead-lettered events — so the metric reflects exactly the events the worker successfully delivered.Why
The counter existed and was documented in the README's
xstreamroll_*metrics, but the only thing that ever incremented it was its own unit test. Wiring the increment into the success path ofpump()(rather than, say, subscribing to the"processed"event in the worker) keeps the metric truthful at the point of delivery: the session already owns the retry/dead-letter decision, so placing the increment next to the successbreakmakes it impossible for a retried or dead-lettered event to double-count or count at all.What was built
xstreamroll-processing/src/:session.tsincrementProcessed()called inpump()right afterawait this.handlers.publish(processed)succeeds (the only change to production code; the import is the only new line).xstreamroll-processing/__tests__/:session.test.tsfast-checkimport to the top to satisfyimport/order.No other files modified — the change is purely additive to one production file plus its test.
Acceptance criteria coverage
messagesProcessedexactly once, including after a transient publish failure that succeeds on retry (session.test.ts— "increments exactly once when a transient failure succeeds on retry", asserts 3 publish attempts → 1 increment)messagesProcessed(session.test.ts— "does not increment messagesProcessed for dead-lettered events", asserts counter unchanged after a dead-letter)GET /metricson the worker's metrics server reflects the incremented value after events flow through a session (the counter lives in the same modulestartMetricsServerreads;metrics.test.tsstill passes and the new session tests readgetMetrics()directly)session.test.ts— new issue Worker messages-processed metric is always zero: incrementProcessed has no production caller #521 describe block)Test plan
cd xstreamroll-processing && npm test— 123/129 passing; the 6 failures are inpipeline.integration.test.tsandworker.integration.test.tsand reproduce identically on the base commit (verified via stash) — pre-existing, unrelated to this changecd xstreamroll-processing && npm run typecheck— cleannpx eslint --config ../eslint.config.js src/session.ts __tests__/session.test.ts— 0 errors, 0 warnings (both files pass the repo's--max-warnings=0gate)jest session.test.ts metrics.test.ts— 37/37 passing (3 new tests)Env vars / Notes
None — no new configuration. The 6 failing integration tests in this workspace are pre-existing (they fail on the base commit too); a separate issue tracks the worker integration-suite state.