chore: simplify, modernize (Go 1.26), update deps#133
Conversation
- writer: add Unwrap() so Hijacker/HTTP2/WebSocket promotion works through the wrapper - writer: default code to 200 in Write() for handlers that skip WriteHeader - plugin: CounterVec with no labels → prometheus.Counter (noFreeWorkers, uptime) - plugin: size-1 buffered stopCh → unbuffered + close() idiom - plugin: capture strconv.Itoa(rrWriter.code) once; drop duplicate call - deps: go get -u all && go mod tidy
|
Warning Review limit reached
More reviews will be available in 32 minutes and 16 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Small reliability/cleanup pass on the http_metrics plugin: it makes the metrics ResponseWriter wrapper transparent to middleware that probes for http.Hijacker/HTTP/2 extensions, fixes the status label for handlers that only call Write, and simplifies a few internal patterns (label-less Counters, unbuffered stop channel via close, status string computed once). Also bumps OpenTelemetry contrib deps.
Changes:
writer.go: addUnwrap()to expose the innerhttp.ResponseWriter, and defaultcodeto 200 insideWrite()whenWriteHeaderwas not called.plugin.go: replace label-less*CounterVecwithCounterfornoFreeWorkers/uptime, switchstopChto unbuffered +close()inStop, hoiststrconv.Itoa(rrWriter.code)into a singlestatusCode, and minor declaration reorder.go.sum: drop old entries forotelhttp v0.68.0andjaeger v1.43.0, keep the upgradedv0.69.0/v1.44.0lines.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| writer.go | Add Unwrap() for http.ResponseController and default status to 200 in Write(). |
| plugin.go | Use scalar Counters, simplify stop signaling with close(), cache status string. |
| go.sum | Remove stale OTel contrib otelhttp/jaeger checksums superseded by the upgrade. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Applied fixes
R (Reliability)
writer.go: AddUnwrap() http.ResponseWriterso Hijacker, HTTP/2, and WebSocket promotion works through the metrics wrapper (was silently breaking those protocols)writer.go: Defaultcodeto 200 insideWrite()— handlers that write a body without callingWriteHeadernow record status 200 instead of -1S (Simplification)
plugin.go:*CounterVecwith no labels replaced withprometheus.CounterfornoFreeWorkersanduptime— cleaner API, no spuriousWith(nil)callsplugin.go: Size-1 bufferedstopChreplaced with unbuffered channel +close()idiom — the correct Go pattern for a one-shot stop signalplugin.go:strconv.Itoa(rrWriter.code)captured once intostatusCode; was evaluated twice (requestCounter + requestDuration)M (Modernization)
plugin.go:var otelVal/tpzero-decls reordered to group related declarations before theif okblock — minor scope clarity improvementDeps
go get -u all && go mod tidy