add support for Gradle prometheus metrics#124
Conversation
andrew
left a comment
There was a problem hiding this comment.
Thanks for splitting this out. The cache hit/miss and storage-operation instrumentation in handleGetOrHead / handlePut is good and matches the pattern already used in handler.go.
I'd like to drop the RecordRequest / statusCapturingResponseWriter part though. LoggerMiddleware in internal/server/middleware.go already wraps every request with a status-capturing writer and measures duration, so gradle requests end up double-wrapped, and more awkwardly this would make gradle the only ecosystem that shows up in RequestsTotal. The right place for RecordRequest is inside that middleware, deriving the ecosystem from the path prefix so every handler is covered at once. I'm happy to do that as a follow-up myself; for this PR could you remove the start/rw/defer block and statusCapturingResponseWriter, and revert the w → rw renames in Routes()?
One small consistency nit while you're in there: the Size call records RecordStorageOperation inside both the success and error branches (and skips it when err == nil && size < 0). Everywhere else the pattern is to record the duration unconditionally right after the call, then if err != nil { RecordStorageError }. Would be good to match that.
The test's before/after delta approach on the global counters is the right way to do it.
Follow-up to #114
This PR adds observability for the Gradle HTTP build cache handler by recording request, cache, and storage metrics through the existing Prometheus metrics package.