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

Custom Request-Id for client side requests #38850

Closed
emanuel-v-r opened this issue Dec 6, 2021 · 13 comments
Closed

Custom Request-Id for client side requests #38850

emanuel-v-r opened this issue Dec 6, 2021 · 13 comments
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares Docs This issue tracks updating documentation feature-diagnostics Diagnostic middleware and pages (except EF diagnostics)

Comments

@emanuel-v-r
Copy link

emanuel-v-r commented Dec 6, 2021

On every new request, aspnetcore creates a new Activity and sets a RequestId, TraceId, and SpanId, and spans it through the logs.
Is it possible to somehow set the ParentId to a UUID coming from the client-side (javascript) via a custom HTTP header?
I understand that if this UUID comes in the Request-Id HTTP header it automatically uses it, but what if it comes in a different custom header?
Thank you.

@rafikiassumani-msft rafikiassumani-msft added the feature-diagnostics Diagnostic middleware and pages (except EF diagnostics) label Dec 6, 2021
@davidfowl
Copy link
Member

Client as in JavaScript or .NET client?

@emanuel-v-r
Copy link
Author

emanuel-v-r commented Dec 6, 2021

Client as in JavaScript or .NET client?

Javascript client, calling this service from the UI. This way we would be able to track down the whole path of a request coming from the browser.
As I understand Request-Id header is already automatically added to requests happening on the .NET side (at least when using HttpClientFactory).

@adityamandaleeka
Copy link
Member

@Tratcher @noahfalk This should be possible now right?

@Tratcher
Copy link
Member

Tratcher commented Dec 8, 2021

It can be customized, I just need to find the example.

Here's the code in hosting, it uses a DistributedContextPropagator.

_propagator.ExtractTraceIdAndState(headers,
static (object? carrier, string fieldName, out string? fieldValue, out IEnumerable<string>? fieldValues) =>
{
fieldValues = default;
var headers = (IHeaderDictionary)carrier!;
fieldValue = headers[fieldName];
},

The only reference I can find is https://devblogs.microsoft.com/dotnet/announcing-net-6-preview-7/#libraries-system-diagnostics-propagators, but @noahfalk would know better.

@cijothomas
Copy link

On every new request, aspnetcore creates a new Activity and sets a RequestId, TraceId, and SpanId, and spans it through the logs. Is it possible to somehow set the ParentId to a UUID coming from the client-side (javascript) via a custom HTTP header? I understand that if this UUID comes in the Request-Id HTTP header it automatically uses it, but what if it comes in a different custom header? Thank you.

This should be possible custom "propagators" (dotnet/runtime#50658). The default propagator respects the W3CTraceContext spec, i.e it looks for "traceparent" header (with a fallback to "request-id" to maintain back compact with legacy header).

@sebastienros
Copy link
Member

This should be possible custom "propagators"

@cijothomas do you have any pointers to some documentation?

@cijothomas
Copy link

This should be possible custom "propagators"

@cijothomas do you have any pointers to some documentation?

Nothing other than the design proposal : dotnet/runtime#50658 (comment)

@rafikiassumani-msft rafikiassumani-msft added the Docs This issue tracks updating documentation label Dec 20, 2021
@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Dec 20, 2021
@ghost
Copy link

ghost commented Dec 20, 2021

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@emanuel-v-r
Copy link
Author

Thank you all for the help here, I will analyze the source code in order to understand how to provide a custom propagator. Some documentation would be nice to have though.

@noahfalk
Copy link
Member

noahfalk commented Jan 4, 2022

@tarekgh @MihaZupan - did any documentation ever get created for the propagators feature? For example it would be ideal to have it documented together with our existing distributed tracing docs.

In the meantime if there are no docs I think the short story is roughly:

  1. Implement a custom version of DistributedContextPropagator. You can use an existing implementation as a guide. If you want to change how new requests have their id populated then you want to change the implementation of ExtractTraceIdAndState. For example if you want the id to be read from a header called "MySpecialIdHeader" and there is no encoding for tracestate you could write:
public override void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState)
        {
            if (getter is null)
            {
                traceId = null;
                traceState = null;
                return;
            }
            getter(carrier, "MySpecialIdHeader", out traceId, out _);
            traceState = null;
        }

If you need to handle the new header in addition to messages that use other more typical headers you can also consider falling back to check other headers when this one isn't present.

Also update the Fields property to enumerate any header names you might want to look up per-request.

  1. Assign a new instance of your propagator to the static property DistributedContextPropagator.Current when your app starts up. This needs to happen before it is captured by the WebHostBuilder here.

@tarekgh
Copy link
Member

tarekgh commented Jan 4, 2022

did any documentation ever get created for the propagators feature? For example it would be ideal to have it documented together with our existing distributed tracing docs.

We have documented the APIs here. Having a conceptual doc for propagators is a good idea too.

@adityamandaleeka
Copy link
Member

Reading through this now, I don't think there's any work in ASP.NET Core for this so I'll close the issue.

@tarekgh You may want to file a tracking item in runtime's docs for the conceptual doc on propagators.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 2, 2022
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares and removed area-runtime labels Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares Docs This issue tracks updating documentation feature-diagnostics Diagnostic middleware and pages (except EF diagnostics)
Projects
None yet
Development

No branches or pull requests

10 participants