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

Add in the HTTP Method + path to the outgoing response log #700

Closed
benlei-gfm opened this issue Jan 28, 2020 · 9 comments
Closed

Add in the HTTP Method + path to the outgoing response log #700

benlei-gfm opened this issue Jan 28, 2020 · 9 comments
Labels

Comments

@benlei-gfm
Copy link

It would be nice if the HTTP method + path can be output in the response log. A lot of times I'm searching for a specific error response on something like SumoLogic, but when I find the error response I don't know what HTTP method + path was called. I have to write another query to find the corresponding incoming request log.

Can you add in the HTTP method + path to the outgoing logs similar to the way incoming request logs have it?

@whiskeysierra
Copy link
Collaborator

Two approaches:

  1. Rely on correlation id as outlined in https://github.com/zalando/logbook/blob/master/docs/scalyr.md
  2. Implement a custom Sink that has full access to both request and response and use a private JsonHttpLogFormatter instance to prepare and format the output as you desire.

@benlei-gfm
Copy link
Author

benlei-gfm commented Jan 28, 2020

  1. I have been, but it requires me to search for it. Moreover if I wanted to use something like SumoLogic to gather all responses for a specific endpoint, it is currently not possible due to the fact that that the information doesn't exist in the outgoing logs. Meaning, I'd have to gather all the incoming requests... and then for each and every request I have to find the corresponding correlation ID'd outgoing response.

  2. I'm currently using the HTTP Logger as it's much more human readable. Besides that, the DefaultHttpLogFormatter is final. I'd prefer to avoid copying over the entire class/rewriting it as a Kotlin object to achieve the same result, as some classes aren't accessible outside of your package (such as the RequestURI class) and it wouldn't be future-proof.

@benlei-gfm benlei-gfm changed the title Can you add in the HTTP Method + path to the outgoing response log? Add in the HTTP Method + path to the outgoing response log Jan 28, 2020
@whiskeysierra
Copy link
Collaborator

I have been, but it requires me to search for it.

Depends. If you log ingestion already provides indexing capabilities that support correlation, then it can copy over fields from one log event to another. Scalyr does this, for example.

Meaning, I'd have to gather all the incoming requests... and then for each and every request I have to find the corresponding correlation ID'd outgoing response.

You wouldn't use linear search with a nested loop join. If you index your incoming requests by correlation id you can change this to a hash join.

I'm currently using the HTTP Logger as it's much more human readable.

There is no reason to stop using that.

Besides that, the DefaultHttpLogFormatter is final. I'd prefer to avoid copying over the entire class/rewriting it as a Kotlin object to achieve the same result

There is no need for inheritance here (personal note: with very few exceptions inheritance is almost never needed). You can do fine with composition. Just create a DefaultHttpLogFormatter and use its public format methods to prepare the output. You are free to modify it before doing any logging.

My suggestion:

Use DefaultSink as a blue print:

@AllArgsConstructor
public final class DefaultSink implements Sink {
private final HttpLogFormatter formatter;
private final HttpLogWriter writer;
@Override
public boolean isActive() {
return writer.isActive();
}
@Override
public void write(final Precorrelation precorrelation, final HttpRequest request) throws IOException {
writer.write(precorrelation, formatter.format(precorrelation, request));
}
@Override
public void write(final Correlation correlation, final HttpRequest request, final HttpResponse response)
throws IOException {
writer.write(correlation, formatter.format(correlation, response));
}
}

change the second write method to something like this:

var requestOutput = request.getMethod() + " " + request.getPath();
var responseOutput = formatter.format(correlation, response);

writer.write(correlation, requestOutput + "\n" + responseOutput);

@benlei-gfm
Copy link
Author

benlei-gfm commented Jan 29, 2020

Depends. If you log ingestion already provides indexing capabilities that support correlation, then it can copy over fields from one log event to another. Scalyr does this, for example.

That's the thing though... I'm not using Scalyr.

You wouldn't use linear search with a nested loop join. If you index your incoming requests by correlation id you can change this to a hash join.

I was just speaking in the abstract - I'm not a sumo logic expert and I'm not sure if it's even possible. Not everyone in my company is an expert on log search and tend to just search by basic terms.

My suggestion

What about the RequestURI.reconstruct(request, builder);? What you've provided wouldn't fully mimic it.

@whiskeysierra
Copy link
Collaborator

I was just speaking in the abstract - I'm not a sumo logic expert and I'm not sure if it's even possible. Not everyone in my company is an expert on log search and tend to just search by basic terms.

If your logging ecosystem only supports grep-like features without any kind of indexing, then yes the default setup won't be fast when querying/searching.

What you've provided wouldn't fully mimic it.

What about request.getRequestUri()?

default String getRequestUri() {
return RequestURI.reconstruct(this);
}

@whiskeysierra
Copy link
Collaborator

For a related discussion see also #691

@benlei-gfm
Copy link
Author

What about request.getRequestUri()?

I guess that could work. Though I'd prefer to put it after the first line instead of the first.

@whiskeysierra
Copy link
Collaborator

For now you'd need to split by newline, insert and join with newlines again. For the JsonHttpLogFormatter we produce an intermediate Map that can be constructed, modified and serialized individually, allowing customization in between. Maybe the DefaultHttpLogFormatter could be enhanced with a new method that returns a List<String>. But that comes with two issues: 1) It requires to specify the format very precisely (which will then be harder to change in the future) and 2) it was originally meant as a human readable formatter and not so much intended for any kind of programmatic interaction. The json format would be better suited for this.

@whiskeysierra
Copy link
Collaborator

I'm closing this for now because I believe that JsonHttpLogFormatter and Sink offer a good enough abstraction and expose enough functionality to customize the default behavior. Feel free to re-open this.

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

No branches or pull requests

2 participants