felixge/httpsnoop

In metrics, do not use 200 as default code

Closed this issue · 5 comments

mitar commented

When doing context cancellation handling, it can happen that the whole handler is never executed (or never gets to writing headers at least). It would be great if I could still use this package to log metrics. And it works. But the issue is that I get 200 as status code. I would prefer if status code would be 0, because then it would be easy to filter those log entries out and see which had context cancellation.

Sounds reasonable. Do you have a proposal for how to accomplish this? The main challenge is that Go writes a 200 response code by default, but IIRC this is not observable from the outside, so this library assumes 200 by default.

mitar commented

I would suggest that the library simply returns 0 if there was no call to WriteHeader. User can then map it to 200. Or if user knows that the handler is calling WriteHeader, then 0 means that it has never been called. This is my situation.

Maybe this could be a configuration option.

Maybe this could be a configuration option.

It would have to be. IMO changing the current implementation would be a breaking change for existing users from my perspective.

Feel free to propose an API for making this configurable (PR or comment).

mitar commented

I made #18.

mitar commented

I remembered another use case where I needed that: websocket upgrade. It was strange to log 200 code when there was a websocket upgrade on the connection.