Skip to content
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

Feature request: More customization points in tracing #1058

Open
cbeck88 opened this issue Aug 1, 2023 · 2 comments
Open

Feature request: More customization points in tracing #1058

cbeck88 opened this issue Aug 1, 2023 · 2 comments
Labels
feature New feature or request

Comments

@cbeck88
Copy link

cbeck88 commented Aug 1, 2023

Is your feature request related to a problem? Please describe.

I have been greatly enjoying the use of warp::trace::request with my web service. It is really great that this can be done with so little boilerplate thanks to warp. However, I am frustrated by being unable to configure it as much as I would like.

The code that I would like to be able to configure is here in the finished_logger function:

fn finished_logger<E: IsReject>(reply: &Result<(Traced,), E>) {

The issue for me is that, sometimes I get logs like this:

2023-08-01T20:13:54.040831Z  WARN request{method=POST path=/login version=HTTP/1.1 remote.addr=127.0.0.1:53508}: warp::filters::trace: unable to serve request (client error) status=423 error=None

What has actually happened is, my handler has reported an error and returned using something like warp::reply::with_status("Helpful error message", StatusCode::LOCKED). However, instead of seeing error=Helpful error message in the tracing logs, I see error = None.

That's in contrast with what happens when the handler actually returns a warp Rejection rather than a Reply. In that scenario it shows a list of rejection reasons at error = in the logs.

(Another approach for my situation might be to try to replace all these with_status results into warp rejections, but at least right now, that's not how I'm doing things. The thinking was that, in these cases, I don't want to go back into the routing system and try other routes and handlers, because at this point I really know what the response should be. It's also not clear to me how I can control the status code when using a custom rejection.)

I think the reason for the difference in these scenarios (reply with an error status code, vs. a rejection) is because at these lines in the finished_logger function:

        let (status, error) = match reply {
            Ok((Traced(resp),)) => (resp.status(), None),
            Err(error) => (error.status(), Some(error)),
        };

When we have a reply, "error" is always set to None, even if the status actually indicates an error. And then when we are in the client_error() scenario later:

        } else if status.is_client_error() {
            tracing::warn!(
                target: "warp::filters::trace",
                status = status.as_u16(),
                error = ?error,
                "unable to serve request (client error)"
            );

we will always report None for the error, even if the reply body contained an error message.

The other frustration I have is that, at least in some situations, I don't want to have client errors reported at warn level, because my logs can be massively spammed if someone is misusing my API. I might rather to set it to debug, run the servers at info level normally, and then change the filtering dynamically to debug for a short time if someone reaches out to me and asks me to investigate something.

In any case, as far as I can tell, I can't customize any of these decisions.

  • finished_logger is in the mod internal and baked into the WithTrace filter type
  • I can't make my own WithTrace because that requires me to have this bit: impl<FN, F> FilterBase for WithTrace<FN, F> but FilterBase is sealed.

(If there's a way to customize this that I'm missing, I would be interested to know)

Describe the solution you'd like

The simplest thing from my point of view would be if I can stick in my own fn(&Result<T, E>) or similar, which would be a member of WithTrace, to be used in place of finished_logger at this point:

.inspect(finished_logger as fn(&Result<Self::Extract, F::Error>))
, but could default to finished_logger perhaps.

Describe alternatives you've considered

I spent a while trying to figure out if there's some way to use this wrap_fn stuff to go around the WrapSealed trait: https://github.com/seanmonstar/warp/pull/693/files and do this all without modifying the library, but I haven't quite figured it out.

Additional context

Thanks

@cbeck88 cbeck88 added the feature New feature or request label Aug 1, 2023
@cbeck88 cbeck88 changed the title More customization points in tracing Feature request: More customization points in tracing Aug 1, 2023
@cbeck88
Copy link
Author

cbeck88 commented Aug 1, 2023

If you'd be willing to take a patch like this I'm happy to try to develop it

@cbeck88
Copy link
Author

cbeck88 commented Aug 1, 2023

I've realized that it's somewhat difficult to access the body text after converting it to warp::Response, so for now I've given up on doing exactly what I described, and now I just add another tracing log whenever my error type is converted to warp::Response, which is another way for me to get that information.

I'd still like to be able to control the log levels here in finished_logger, but that's possibly a much smaller scope of change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant