Skip to content

Commit

Permalink
roll up HTTP success and status code group metrics (#958)
Browse files Browse the repository at this point in the history
Following up on #955, I missed that our observability system would also
make it too hard to roll up the various status codes into sums.

So, we add some per-hundred status code metrics, and a
`response.success` metric that includes both 2xx and 4xx responses. The
4xx count as success because (as the [HTTP docs
say](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status)) 4xx
status codes are client errors. Counting someone's system accidentally
using the wrong credentials shouldn't against our success rate.
  • Loading branch information
jmhodges committed Aug 29, 2024
1 parent 236ee60 commit 88d625a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
15 changes: 15 additions & 0 deletions stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ func (w *statsdWriter) Write(b []byte) (int, error) {

func (w *statsdWriter) WriteHeader(statusCode int) {
if w.headerWritten.CompareAndSwap(false, true) {
switch {
case statusCode >= 200 && statusCode < 300:
w.stats.Incr(fmt.Sprintf("%s.response.status.2xx", w.metricPrefix), nil, 1)
w.stats.Incr(fmt.Sprintf("%s.response.success", w.metricPrefix), nil, 1)
case statusCode >= 300 && statusCode < 400:
w.stats.Incr(fmt.Sprintf("%s.response.status.3xx", w.metricPrefix), nil, 1)
case statusCode >= 400 && statusCode < 500:
w.stats.Incr(fmt.Sprintf("%s.response.status.4xx", w.metricPrefix), nil, 1)
// 4xx is a success code for availability since this is
// generally folks messing up their authentication. Still want
// to have these on a dashboard as a double check, though.
w.stats.Incr(fmt.Sprintf("%s.response.success", w.metricPrefix), nil, 1)
case statusCode >= 500 && statusCode < 600:
w.stats.Incr(fmt.Sprintf("%s.response.status.5xx", w.metricPrefix), nil, 1)
}
w.stats.Incr(fmt.Sprintf("%s.response.status.%d", w.metricPrefix, statusCode), nil, 1)
w.ResponseWriter.WriteHeader(statusCode)
}
Expand Down
6 changes: 6 additions & 0 deletions stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ func TestStatsResponseWriterWritesResponseMetricOnce(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStats := mockstatsd.NewMockClientInterface(ctrl)
mockStats.EXPECT().Incr("myhandler.response.status.4xx", []string(nil), 1.0).Times(1)
mockStats.EXPECT().Incr("myhandler.response.success", []string(nil), 1.0).Times(1)
mockStats.EXPECT().Incr("myhandler.response.status.400", []string(nil), 1.0).Times(1)

recorder := httptest.NewRecorder()
Expand All @@ -34,6 +36,8 @@ func TestStatsResponseWriterWritesToHeaderOnWrite(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStats := mockstatsd.NewMockClientInterface(ctrl)
mockStats.EXPECT().Incr("myhandler.response.status.2xx", []string(nil), 1.0).Times(1)
mockStats.EXPECT().Incr("myhandler.response.success", []string(nil), 1.0).Times(1)
mockStats.EXPECT().Incr("myhandler.response.status.200", []string(nil), 1.0).Times(1)

recorder := httptest.NewRecorder()
Expand All @@ -49,7 +53,9 @@ func TestWrappingStatsResponseWriteWritesAllMetrics(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
mockStats := mockstatsd.NewMockClientInterface(ctrl)
mockStats.EXPECT().Incr("inner.response.status.5xx", []string(nil), 1.0).Times(1)
mockStats.EXPECT().Incr("inner.response.status.500", []string(nil), 1.0).Times(1)
mockStats.EXPECT().Incr("wrapper.response.status.5xx", []string(nil), 1.0).Times(1)
mockStats.EXPECT().Incr("wrapper.response.status.500", []string(nil), 1.0).Times(1)

recorder := httptest.NewRecorder()
Expand Down

0 comments on commit 88d625a

Please sign in to comment.