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.

Sign up for GitHub

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

Jump to bottom

Header added to request in rhai does not propagated into logs trace_id field #4892

Closed
kindermax opened this issue Apr 2, 2024 · 5 comments · Fixed by #5071
Closed

Header added to request in rhai does not propagated into logs trace_id field #4892

kindermax opened this issue Apr 2, 2024 · 5 comments · Fixed by #5071
Assignees
@BrynCooke
Labels
raised by user

Comments

@kindermax
Copy link
Contributor

Describe the bug

Router version: v1.43.1

I use rhai to insert a x-trace-id header into request. Then I expect value of this header to be a trace_id in logs.

To Reproduce

Here I use x-trace-id to propagate header value to logs.
If x-trace-id present in headers from browser request - it works fine. But if I add x-trace-id header in rhai script - value does not propagated.

telemetry:
  exporters:
    tracing:
      propagation:
        request:
          header_name: x-trace-id
    logging:
      stdout:
        enabled: true
        format:
           json:
             display_level: true
             display_timestamp: true
             display_trace_id: true
             display_span_id: true

rhai script

fn supergraph_service(service) {
    let adapt_request_id = |request| {
        if request.headers.contains("x-request-id") {
            // x_request_id is a uuid4
            let x_request_id = request.headers["x-request-id"];
            x_request_id.replace("-", "");
            print(`x-request-id: ${x_request_id}`);
           // x-trace-id is just a copy of x-request-id without dashes
           // so router can parse it properly
           // we then restore it to uuid format in the logs processor
           request.headers["x-trace-id"] = x_request_id;
       }
    };

    service.map_request(adapt_request_id);
}

Expected behavior

I expect that rhai-created header propagated to trace_id field in logs.
@Geal Geal assigned Geal and BrynCooke and unassigned Geal Apr 22, 2024
@BrynCooke
Copy link
Contributor

Hi @kindermax,
I'm afraid that what you are trying to do can't possibly work. Extracting the trace ID and constructing the root span is necessarily the first thing that should happen in the router. (There are some things that happen before but we are working to fix this).

I think we would accept a PR that simply strips out dashes in CustomTraceIdPropagator.
Alternatively is this driven by use of AWS X-ray? If you can use a standard propagator and trace_id format it would be prefereable.

@kindermax
Copy link
Contributor Author

kindermax commented Apr 22, 2024 •
edited

Hi, unfortunately in my case uuid4 x-request-id is a company standard and generated by our nginx so I can not change its format.
I think stripping dashes in CustomTraceIdPropagator would be enough in my case since I just need its value in my logs. Then I can do the reverse thing (adding dashes again) in my log processor.
If you ok with it, I can try to make a pr

@BrynCooke
Copy link
Contributor

That would be great. Make sure to add some sort of test.
I think it's OK to strip dashes out as it's not as if we can use anything with dashes in.

@kindermax kindermax mentioned this issue May 3, 2024
Merged
6 tasks
@abernix abernix mentioned this issue May 6, 2024
Open
@abernix
Copy link
Member

abernix commented May 6, 2024

@kindermax Do you think that the PR #5071 will resolve this satisfactorily?

@kindermax
Copy link
Contributor Author

kindermax commented May 6, 2024 •
edited

@abernix well, partially yes. If I put a log processor after router like this router | log_processor, then I will be able to restore original trace_id. Sure it wont work if the original trace_id were without dashes, because I will add dashes when I shouldn't. Alternatively, if there was a way just to add my header value to some field in logs (its a paid feature as far as i know) I would be able to just extract it in log processor.

@IvanGoncharov IvanGoncharov mentioned this issue May 21, 2024
Merged
@smyrick smyrick mentioned this issue May 21, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@BrynCooke BrynCooke

Labels
raised by user
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Strip dashes from trace_id in CustomTraceIdPropagator kindermax/router
4 participants
@Geal @BrynCooke @abernix @kindermax