-
-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Request & Response Logger #265
Comments
couple things to add and question. Please allow for disablement of one logger or other. In an environment that uses a saas product for their monitoring infra logging both the request and the response can cause a lot of costs! Also I assume the default should be with slog? |
One more though on request id here. What are the thoughts checking if the header of |
Yes, yes and yes! Good ideas :) Also, if the request can be DEBUG and the response INFO, it would be nice ! |
Yeah I agree with you after playing around a bit tonight. The request log should just be debug. Trying to make everything configurable isn't worth the squeeze. It ends up just being easier to create your own logger middleware. |
instead of |
But it would mean in another module and not included by default, right? |
Hmmm, the more I think about this the more I’m realizing I’m thinking about ding too much. yeah, let’s just to the default that you described. With a simple on/off. I am imagining middlewares/logger could exist eventually, but that would be a separate effort outside of fuego core. I think this default logger should aim for being just enough. |
Yeah just a basic logger will be enough. For advanced use cases, users won't be afraid to create their own. |
Hey @dylanhitt @EwenQuim Am I OK to work on this issue? One question about configurability - do we want to selectively enable/disable either the resp or req logs? Or just a global flag that turns off this middleware completely, so anyone is free to impl their own? |
I would say just a global on/off. But if you find a way to do both without too much downsides, you can do it. I'm open to suggestions, do as you wish! :) |
@jwtly10 I played with the idea of doing it, but couldn't really find a pattern I liked for it. Basically what Ewen said if you can figure out way to do it cleanly please too. Here's some draft PRs of what my approach was. If not I liked Ewen's idea of setting the request log to Debug. |
Had a first stab at it, pretty much just building on what you did @dylanhitt Added some tests to show the API, I've tried to keep it simple and inline with existing options |
Can I try solving this issue? |
I'd like to take this issue. |
Let me try this one! |
I’m interested in this one. |
Mind if I try this one? |
closed with #313 |
Is your feature request related to a problem? Please describe.
A middleware, enabled by default but desactivable via a server option, that logs:
You can create a new
default_middlewares.go
and the option to activate/desactivate it should be in theserver.go
fileThe text was updated successfully, but these errors were encountered: