Skip to content

feat(router)!: log numeric duration_ms and summarize hijacked connections at handoff - #142

Merged
Suhaibinator merged 1 commit into
mainfrom
feat/numeric-duration-hijack-summary
Sep 29, 2026
Merged

Suhaibinator merged 1 commit into
mainfrom
feat/numeric-duration-hijack-summary

Conversation

@Suhaibinator

Copy link
Copy Markdown
Owner

Summary

Numeric duration. The request summary logged zap.Duration, and the text of that field depends on the application's EncodeDuration. go-common uses StringDurationEncoder, which writes values like "1h25m30s" and "504.96ms". Open Splunk, and SPL in general, can't sort or aggregate those, so sort num(duration) gave meaningless order. The summary now writes duration_ms as a float64 number of milliseconds, whatever encoder the app uses.

Hijacked connections. WebSocket upgrades were summarized when the handler returned. That meant the summary recorded the whole connection lifetime (hours), and every long-lived socket was logged at Warn as a slow request. Now metricsResponseWriter.Hijack writes the summary at a successful hijack, with hijacked: true added, and the end-of-request summary is skipped for that connection. A failed hijack keeps the normal end-of-request summary.

Breaking changes

  • The request summary field duration is replaced by duration_ms.
  • logkeys.Duration is replaced by logkeys.DurationMS. logkeys.Hijacked is new.

Notes

  • status for a hijacked summary is whatever the router saw before the handoff. That is 101 if the handler called WriteHeader(101) first (coder/websocket does), otherwise the default 200 (gorilla writes its 101 straight to the hijacked conn).
  • Not in this PR: the metrics middleware's request_latency_seconds still records the full hijacked-connection lifetime. ratelimit's window/reset_duration and the timeout log's timeout still use zap.Duration.

Test plan

  • go test ./..., go test -race on the new tests
  • golangci-lint run, go vet ./...
  • go test ./internal/doccheck, go build ./examples/...
  • New tests: summary written once at handoff while the socket is still open, and not again at close; duration_ms is a float64; a failed hijack keeps the end-of-request summary.

🤖 Generated with Claude Code

…ions at handoff

The request summary logged zap.Duration, whose rendering depends on the
application's EncodeDuration. With StringDurationEncoder it produced values
like "1h25m30s" and "504.96ms" that log search tools cannot sort or
aggregate. Emit duration_ms as a float64 number of milliseconds instead.

Hijacked connections (WebSocket upgrades) were summarized when the handler
returned, so the summary reported the full connection lifetime and logged
every long-lived socket as a slow Warn request. Write the summary at a
successful hijack, mark it with hijacked=true, and skip the end-of-request
summary for that connection.

BREAKING CHANGE: the request summary field "duration" is replaced by
"duration_ms", and logkeys.Duration is replaced by logkeys.DurationMS.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Sep 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.93%. Comparing base (8612569) to head (88bdfa1).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #142   +/-   ##
=======================================
  Coverage   99.93%   99.93%           
=======================================
  Files          31       31           
  Lines        3180     3193   +13     
=======================================
+ Hits         3178     3191   +13     
  Misses          2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Suhaibinator
Suhaibinator merged commit 9f749bc into main Sep 29, 2026
11 checks passed
@Suhaibinator
Suhaibinator deleted the feat/numeric-duration-hijack-summary branch September 29, 2026 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant