Aspnetcore: Add a way to configure the IHttpRequestIdentifierFeature to read a configurable header

[sirajmansour]

I am facing the same dilemma as https://github.com/aspnet/KestrelHttpServer/issues/2153.

Setting the trace identifier on the HttpContext isn't particularly helpful for logging because the request start scope was started before any middleware is called, so all logs use the traceidentifer set by Kestrel.

Basically, we force all incoming requests to our services to have a correlation-id header as we want to track the whole flow coming from the front-end or any other consumers of our APIs. We also prefer using X-Correlation-Id instead of Request-Id.

Although it seems a workaround is mentioned there by @davidfowl, i think it is valuable having that option configurable at the early stages of creating the webhost.

It would great to be able to read from a header or (any other delegate/provider). And propagating that change across to HostingApplicationDiagnostics.cs instead of setting it always to look for Request-Id header.

Also making the property name in the log name configurable is valuable too as it is also statically set to CorrelationId as shown in https://github.com/aspnet/Hosting/blob/dev/src/Microsoft.AspNetCore.Hosting/Internal/HostingLoggerExtensions.cs or RequestId - for 2.0 - which has the value of the httpcontext.TraceIdentifier that we manually set from our x-correlation-id header.

[davidfowl]

Is your goal to add extra state to the logging scope or something else?

[sirajmansour]

Well the state already exists, it is being defaulted in the hosting scope to CorrelationId (or RequestId in 2.0) but it is either generated by kestrel purely, or read from a set-in-stone header which is Request-Id. I'm suggesting that becomes more flexible and configurable.

[sirajmansour]

I also don't want to widen this discussion a lot, but i've read here that the Request-Id header is used to flow the correlation id with outgoing requests from HttpClient to correlate requests going to other micro-services, which is also very important and related to this since once again the source it reads the correlation id from is not configurable; it will always come from the Request-Id header.

[davidfowl]

Well the state already exists, it is being defaulted in the hosting scope to CorrelationId (or RequestId in 2.0) but it is either generated by kestrel purely, or read from a set-in-stone header which is Request-Id. I'm suggesting that becomes more flexible and configurable.

But that's not what I asked. I'm asking what you specifically are looking for. I'm aware of what our current infrastructure does but it may not work as you'd expect. All of that logic doesn't just work, it requires a diagnostic listener to be wired up which may be undesirable.

I'd like to understand what your trying to achieve (I have some ideas but I don't want to assume anything). Here are some of the things I can think of:

• Ability to set the TraceIdentifier for a request
• Ability to add extra properties in the default request logging scope created by hosting
• Ability to add headers outgoing http client requests (all of them or just specific ones?)
• Ability to get the "current activity" from any where in the code.

/cc @glennc

[sirajmansour]

The main use cases are:

• Logging the correlation id coming from the headers (or any other configurable source) using the default request logging scope created by hosting. whether we do that by allowing people to add a new property in that scope or configure the name of an existing one this is an implementation detail i guess.

• Add the captured correlation-id to a header of outgoing http client requests.

[sirajmansour]

@davidfowl @glennc what are your thoughts on this ?

[ngbrown]

I'm here because I was setting context.TraceIdentifier in a middleware, but it was always missing the very first "Request starting GET /somepath" log message because that is outputted before the middleware pipeline starts. This was setting the RequestId scope for all the rest of the log messages based on an incoming header from a preceding service (API Gateway).

I see that in ASP.Net Core 2.1 preview, the log message scope will add an CorrelationId, but it doesn't appear that the rest of the code can easily access the correlation id or that it is set if the incoming request is not setting a Request-Id header.

So, the difference between RequestId and CorrelationId should be clarified, and there needs to be a single scope key that can span multiple layers of services. I think this means that both are always set in the logging scope and CorrelationId defaults to be the same as RequestId if there is no incoming Request-Id header.

I don't mind setting outgoing HTTP calls with my own headers, but with the above logic, it would be context.TraceIdentifier only if the Request-Id header isn't set in the request.

[davidfowl]

Logging the correlation id coming from the headers (or any other configurable source) using the default request logging scope created by hosting. whether we do that by allowing people to add a new property in that scope or configure the name of an existing one this is an implementation detail i guess.

We do this by default in 2.1. If you set the "Request-Id" header (not configurable yet). It gets stamped onto the logging scope as "CorrelationId".

Add the captured correlation-id to a header of outgoing http client requests.

That's where this currently comes in https://github.com/dotnet/corefx/blob/c082d21361608e3cc2ea3cddd90c2a0306828002/src/System.Net.Http/src/HttpDiagnosticsGuide.md. Basically the right things for outgoing requests light up automagically if you add a diagnostic source subscription.

I don't mind setting outgoing HTTP calls with my own headers, but with the above logic, it would be context.TraceIdentifier only if the Request-Id header isn't set in the request.

@ngbrown have you read https://github.com/dotnet/corefx/blob/c082d21361608e3cc2ea3cddd90c2a0306828002/src/System.Net.Http/src/HttpDiagnosticsGuide.md.

[sirajmansour]

@davidfowl yes i know it is. But my point is about making it configurable, such as if you set "MyWeirdCustomHeader" header, it gets stamped onto the logging scope as "MyWeirdCustomCorrelationId" if i have that configuration in place.

[davidfowl]

Oh yeah I agree it really should be configurable.

[ngbrown]

@ngbrown have you read ...HttpDiagnosticsGuide.md.

Yes, it doesn't mention anything about a browser client hitting the first service. If a request comes in to an API gateway and is scattered/gathered or even goes through multiple layers, that first endpoint hit as the gateway should be included in the CorrelationId for logging. But it is set so early in the chain (before middleware) there's not a clean way to set it if there's no incoming header.

Thus, it would make sense for the CorrelationId to default to something such as the RequestId, or even a GUID. Or make a service point that can generate them (always, or maybe if the Request-Id header isn't set).

[sirajmansour]

@davidfowl cool, this pretty much sums up what'd be nice to see in a future release.

That and a change that ensures having the logging scope of the very first request log - Request starting at /somepath - is read from a configurable source without having to implement a custom HttpContextFactory.

[davidfowl]

We're going to tackle this properly in 3.0

[muratg]

@davidfowl, tentatively assigning this to you.

[anurse]

Epic https://github.com/aspnet/AspNetCore/issues/8924

[anurse]

@lmolkova will investigate this further and break down into specific work items for ASP.NET Core.

[davidfowl]

With the way we have things planned currently:

• TraceIdentifier is always created by the server, it's settable but it's pretty hard to plug in early enough to affect the logging scope (which I assume is the goal). It will continue to be added to the request log scope as "RequestId".
• When the Activity is active (this will be on by default in 3.0 if logging is on), it will generate an Id on every request and it will set the parent id using the using the "Request-Id" header (legacy) or the new "traceparent" header (from the W3C trace-context format (see https://w3c.github.io/trace-context/)).

This means we'll have 2 properties on the request logging scope by default ("RequestId" and "ActivityId").

Coming back to the original request from @sirajmansour:

As for configurability of the above, it seems there are 2 things that could be configurable:

1. The incoming header that represents the "parent id"
2. Control over changing the "current id" for the request.

We don't allow you to reasonably change any of those today but it would be good to understand what you need to change here. We also currently don't have a "passthrough" concept where the "parent id" and "current id" are the same, there's always a hierarchy.

What do you want to configure and where do you want it to go?

For outbound requests, HttpClient has built in support for creating a new activity and setting the outgoing header "traceparent".

[sirajmansour]

When the Activity is active (this will be on by default in 3.0 if logging is on), it will generate an Id on every request and it will set the parent id using the using the "Request-Id" header (legacy) or the new "traceparent" header (from the W3C trace-context format (see https://w3c.github.io/trace-context/)).

It would be good to clarify some things and refresh my memory around this.

"It will generate an Id on every request" ?

Do you mean this will be Activity.Current.Id ? If yes, i'm assuming Activity.Current.ParentId will be set from the Request-Id or traceparent headers as you mentioned above. ?

And i assume you are referring to those two further down as "current id" and "parent id" ?

Basically I think what i want to be configurable hasn't changed.

The incoming header that represents the "parent id"

This seems to tick one of the boxes.

The other thing, would be to control how this incoming header value affects the logging scope (iow, how it appears in the logs).

For outbound requests, HttpClient has built in support for creating a new activity and setting the outgoing header "traceparent".

I'm not sure i have enough context around this, i'll have to dig for an example.

[davidfowl]

We’ve punted this issue from as we’re focusing on supporting w3c and having it on by default. That means if you set the incoming request id, or trace parent it’ll be the parent Id and if logging is on, the scope will contain both trace identifier as RequestId and ActivityId as the full ActivityId (including parent id). Now that we have a standard for these things id expect people to trend that direction (traceparent being the standard header and traceid and span is being the standard properties that appear in logs).

[fazil1987]

Oh yeah I agree it really should be configurable.

@davidfowl so is Request-Id header name currently configurable ? if so, how do i do that as we are not planning to move to w3c standard any time soon. Please reply

[davidfowl]

The name isn't configurable no. Anything custom requires custom logic in HttpClient (a delegating handler) and ASP.NET Core (a middleware)

[alefranz]

Hi @fazil1987 ,
if you are looking for the ability to pass a incoming header down to the HttpClient, you can use the HeaderPropagation middleware which does exactly that (see example).

For the logging part, you can create a middleware at the start of the pipeline to get the header and add it to the logger scope.