Logging middleware #52
Conversation
src/app.rs
Outdated
| App { | ||
| data, | ||
| router: Router::new(), | ||
| middleware: Vec::new(), | ||
| middleware: vec![Box::new(logger)], |
There was a problem hiding this comment.
Let's hold off on setting up default middleware just yet -- maybe open an issue on it so we can discuss how we want to approach it?
There was a problem hiding this comment.
The reason I went with a default middleware was because I felt that the logging provided by this is something that a framework should provide at the least. Is that a reasonable expectation from Tide? If it is, then where will the middleware loaded? As a part of the initial configuration?
This leads to my next question being what will app configuration look like? But I think that is being discussed over at #5
There was a problem hiding this comment.
OK, that's fair enough -- I certainly agree that this should be part of the default middleware set.
src/lib.rs
Outdated
| @@ -17,6 +17,7 @@ pub mod body; | |||
| mod endpoint; | |||
| mod extract; | |||
| pub mod head; | |||
| mod logger; | |||
There was a problem hiding this comment.
We should consider putting this into a submodule of middlware and exposing it publicly that way.
src/logger.rs
Outdated
| /// | ||
| /// Only used internally for now. | ||
| pub(crate) struct RootLogger { | ||
| // drain: dyn slog::Drain, |
There was a problem hiding this comment.
Should clean up this commented-out line.
There was a problem hiding this comment.
Also, if/when we expose this publicly, we should bikeshed the name a bit. (No strong opinion here)
There was a problem hiding this comment.
Yeah, the name was something I went back and forth on for a while. I decided to stick with RootLogger as the idea was that users would use child loggers of this instance.
|
OK, I think we can land this as-is, after a rebase! |
* No interface to log data currently present. * Add a logger type which wraps over slog logger. * Hook into the request cycle.
* This is in order to convey the idea that this is the RootLogger off of which child loggers should be instantiated. Fix lint warnings
* Remove unecessary types * Remove unecessary request method implementation
bIgBV
force-pushed
the
logging-middleware
branch
from
77da812 to
7431da8
Compare
* Move logging.rs into middleware directory
|
Thank you! |
This PR is to establish a base to build the logging infrastructure upon. The discussion is still ongoing in #8 , but this PR introduces a basic logging instance to start displaying information regarding the requests being served by the application. The output of running an application now shows up in the terminal as follows:
As you can see, the method, path and the status code of a request are logged to the terminal. This provides immediate feedback for the user.
Right now I'm instantiating an instance of the
SimpleLoggermiddleware and adding it to the list of middlewares by default.