register fns for custom request-derived logging units #1749
Conversation
|
For this to be considered, I need to know how this is different from just using a middleware function ( |
Codecov Report
@@ Coverage Diff @@
## master #1749 +/- ##
==========================================
+ Coverage 53.82% 53.98% +0.15%
==========================================
Files 126 125 -1
Lines 12073 12104 +31
==========================================
+ Hits 6498 6534 +36
+ Misses 5575 5570 -5
Continue to review full report at Codecov.
|
|
Ah, I didn't think about using If I have a middleware logging incoming requests and a Actix app #[actix_web::main]
async fn main() -> std::io::Result<()> {
std::env::set_var("RUST_LOG", "actix_web=info");
env_logger::init();
HttpServer::new(|| {
App::new()
// enable logger
.wrap(middleware::Logger::new(r#"%a "%r" %s %b "%{Referer}i" "%{User-Agent}i" %T "#).register_request_closure(|req: &ServiceRequest| {
req.headers().get("Test").unwrap().to_str().unwrap().to_owned()
}))
.wrap_fn(|req, srv| {
let header = req.headers().get("Test").unwrap().to_str().unwrap().to_owned();
let fut = srv.call(req);
async move {
let res = fut.await?;
println!("{:#?}", header);
Ok(res)
}
})
.service(web::resource("/").to(|| async { "Hello world!" }))
})
.bind("127.0.0.1:8080")?
.run()
.await
}Bash script #!/bin/bash
curl -H 'Test: test1' localhost:8080 && echo "done1" &
curl -H 'Test: test3' localhost:8080 && echo "done2" &
curl -H 'Test: test2' localhost:8080 && echo "done3" &
waitActix-web Output Here you can see the headers being printed in a way that causes a disjoint between the middleware logger and the |
|
Alright, there is a reasonable use case for this to exist as you've demonstrated with log lines being disjoint. I think there is a more flexible way to achieve this: being able to define custom replacements to use in the format string. We can currently use Proposal: Allow the format syntax Logger::custom_request_replace(&mut self, label: &str, f: Fn(&ServiceRequest) -> String) -> Self(Open to better method name.) |
…esult is inserted in log line
|
Okay above proposal has been implemented. Thanks for the outline; this is working quite nicely. You may notice that I have removed Firstly when trying to add the I tried messing around with lifetimes in order to create a more "specific" lifetime but had no luck in getting this to work. I also stumbled across this issue whilst researching this problem. This made my wonder if the problem is caused by my shallow knowledge of lifetimes or if its a rust issue. In any case I noticed that |
|
Ive added most of the changes you outlined. Thanks for the assistance with that; was nice to go a little deeper into the world of closures. Right now in Also didn't add any test for capturing env for the |
|
np.. nice update. actix-cors managed to avoid require the Clone bound on the closure because it's being put in a Rc wrapper anyway (see https://github.com/actix/actix-extras/blob/master/actix-cors/src/builder.rs#L185). I'd need to have a mess with the code but it's forsure possible. still would like that enum variant to elude to it referencing Requests, since it's public this will be locked as a name for at least the next major version |
|
Quick Q for @actix/contributors? Strictly this is breaking change to anyone who is exhaustively matching over the Logger format types. I would consider it harmless since it has always been hidden from docs. Ergo, I would propose our policy should be server exemptions for doc(hidden) items. Can I get a 👍 or 👎 on that? |
robjtede
changed the title
PR Type
Feature
PR Checklist
Overview
Adds possibility to register a closure to be executed in request middleware logging. Closure is implemented to work in the request scope and thus takes
ServiceRequestas input.Our use case:
We wanted to log the UserID associated with all incoming requests to our API. UserID's in our case are stored inside JWT tokens. This means we cannot log the header directly but need to make some operations on it to get the internal encoded UserId. By allow the injection of this closure we can do the following: