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 a header for distinguishing the services in logs #133

Closed
DawidNiezgodka opened this issue Oct 27, 2022 · 2 comments · Fixed by #131
Closed

Add a header for distinguishing the services in logs #133

DawidNiezgodka opened this issue Oct 27, 2022 · 2 comments · Fixed by #131
Assignees
Labels
area/core Relates to core functionality of Quick type/enhancement New feature or request

Comments

@DawidNiezgodka
Copy link
Contributor

DawidNiezgodka commented Oct 27, 2022

To provide more context to log statements and facilitate the process of discerning them (what belongs where?), a X-Request-ID Header can be set.

A possible implementation involves the use of the HttpFilter and an Interceptor.

Furthermore, the functionality of setting a header should be configurable (on-off).

Implementation

If I understood correctly, the path from a request to a response is the following:
Req (1) -> Filter (2) -> Interceptor (3) -> Controller (4) -> Interceptor (5) -> Filter (6) -> Response (7)

  1. We might filter each incoming request and add the X-Request-ID Header with a unique, randomly generated ID. However, this can only be accomplished with a challenge, as Micronaut does not provide the possibility to set additional headers to a request. One can only set attributes, which does not solve the problem because the attributes can only be accessed from the response level.
  2. The second idea is to solely use responses as a source of this header:
@Filter(Filter.MATCH_ALL_PATTERN)
public class XReqIdHeaderFilter implements HttpServerFilter {

    public static final String HEADER = "X-Request-ID"; 

    public static final IdGenerator IdGenerator;

    @Override
    public Publisher<MutableHttpResponse<?>> doFilter(final HttpRequest<?> request, final ServerFilterChain chain) {
        return Flowable.fromPublisher(chain.proceed(request))
        .map(response -> {
            response.header(HEADER, IdGenerator.generate());
            return response;
        });
    }
}

The id could be later retrieved to extend the log statement with the ID information.
The problem is that we do this at the late stage and can't track as much info as we could.

  1. We could also add the header to a request when we create it. For example, it DefaultMirrorRequestManager. However, this solution is not robust as we must manually track all places in code where we create requests.
  2. Yet another approach is to use an interceptor. The advantage is the possibility of changing the request directly. Furthermore, we follow what happens from an early state (3), and thus, we can receive more info:
public class RequestHeaderInterceptor implements Interceptor {

    public static final String HEADER = "X-Request-ID";

      public static final IdGenerator IdGenerator;

    @NotNull
    @Override
    public Response intercept(final Chain chain) throws IOException {
        Request reqWithHeader = chain.request().newBuilder()
            .header(HEADER, idGenerator.generate()).build();
        return chain.proceed(reqWithHeader);
    }
}
@DawidNiezgodka DawidNiezgodka added the area/tooling Relates to tooling around Quick, e.g. justfile label Oct 27, 2022
@DawidNiezgodka DawidNiezgodka added this to the Tracing and logging milestone Oct 27, 2022
@DawidNiezgodka DawidNiezgodka self-assigned this Oct 27, 2022
@DawidNiezgodka DawidNiezgodka moved this to Todo in Quick Oct 27, 2022
@torbsto
Copy link
Contributor

torbsto commented Oct 28, 2022

  1. [...] However, this can only be accomplished with a challenge, as Micronaut does not provide the possibility to set additional headers to a request.

Why do you think so? The following should achieve it:

@Filter(Filter.MATCH_ALL_PATTERN)
public class XReqIdHeaderFilter implements HttpServerFilter {
    public static final String HEADER = "X-Request-ID";

    @Override
    public Publisher<MutableHttpResponse<?>> doFilter(final HttpRequest<?> request, final ServerFilterChain chain) {
        return chain.proceed(request.mutate().header(HEADER, UUID.randomUUID().toString()));
    }
}
  1. The second idea is to solely use responses as a source of this header:

I don't see the benefit here. This would mean only the caller has the ID, but we still wouldn't have a mechanism to find its logs in the callee.

The problem is that we do this at the late stage and can't track as much info as we could.

Why can't we? Sure, we would need to keep some sort of context alongside all calls, but I wouldn't mind.

Yet another approach is to use an interceptor. The advantage is the possibility of changing the request directly. Furthermore, we follow what happens from an early state (3), and thus, we can receive more info:

This is basically Option 2, but instead the callee sets it. It still has the same problem, because the callee generates the ID only after having done all the work.


I would suggest looking into option 1 again and let the filter always set the header if it doesn't exist yet. This means the first time we get a request (in the manager or gateway), we attach an ID and use it for all subsequent requests in the system.
We could further set the header in the cli or other clients, to be able to correlate client and server logs.

@torbsto
Copy link
Contributor

torbsto commented Oct 28, 2022

If I understood correctly, the path from a request to a response is the following:
Req (1) -> Filter (2) -> Interceptor (3) -> Controller (4) -> Interceptor (5) -> Filter (6) -> Response (7)

This is not completely correct. You have to differentiate between incoming and outgoing requests.

Incoming:
-> Request -> Filter -> Controller -> Filter -> Response

Inside the controller you can send outgoing requests to other services:
-> Interceptor -> [... ] -> Interceptor

Inside the brackets happens the flow for incoming requests, but in a different service.
Overall, filters and interceptors have the same functionality, but filters are used by the server and interceptors by the client. But a server may become a client if it sends a request to other servers.

@torbsto torbsto added type/enhancement New feature or request area/core Relates to core functionality of Quick and removed area/tooling Relates to tooling around Quick, e.g. justfile labels Oct 28, 2022
@raminqaf raminqaf moved this from Todo to In Progress in Quick Nov 2, 2022
@DawidNiezgodka DawidNiezgodka moved this from In Progress to In-Review in Quick Nov 8, 2022
Repository owner moved this from In-Review to Done in Quick Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core Relates to core functionality of Quick type/enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants