-
Notifications
You must be signed in to change notification settings - Fork 759
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 propagation middleware and message handler #526
Comments
Marking for up for grabs. If you're interested in this please continue the discussion here first so we can work out details. |
I’d be potentially interested in helping out with this. We currently have some in-house bespoke handlers to do header forwarding, so a configurable, built-in way to do the same thing would be great. |
Hi @rynowak - Depending on timescales I'd quite like to throw in on this one since you may be after similar to my library. I've been meaning to dig into the Activity bits at some point too. I've just recently added a handler within an internal project that applies the correlation ID using my library which works quite nicely. |
Ah, @martincostello beat me to the punch! Clearly we're both watching notifications from this repo!! :-) |
I’m sure multiple heads thinking about it are better than one! |
cool - I'd definitely look forwards to some contributions from you both. I think I'd also like to get some input from @davidfowl and @glennc and use this issue to discuss design proposals as well as scenarios. |
Here's the sample I wrote https://gist.github.com/davidfowl/c34633f1ddc519f030a1c0c5abe8e867. This is just generic header propagation not tied to any specific implementation of activities or id generation etc. |
Would simple transformations be on the table for this feature as well? We do a few things internally where we map one header to another name when we pass it on. An example would be something like |
I was thinking the exact same thing as @martincostello. It's something we recently needed to do and a few people asked about adding that to the CorrelationId library too. Also, some people ask about being able to control this per endpoint. In that respect, something that can be added per logical client on the HttpClientBuilder would be useful too. |
See https://gist.github.com/davidfowl/c34633f1ddc519f030a1c0c5abe8e867#file-headerpropagation-cs-L24
As for the mapping I'd like to see a couple of real examples. I believe there's a need there but I don't know the extent of the mapping capabilities required. Is it a simple services.AddHeaderPropagation(options =>
{
options.Mapping = (httpContext, httpRequestMessage) =>
{
// Write code to propagate headers here
};
}); |
I like this idea of it sort of being "layered" so you can go low-level and custom if you really want to. Something super-simple could then be powered off that with a helper overload, something kinda like: public static IServiceProvider AddHeaderPropagation(this IServiceProvider services, NameValueCollection mapping)
{
return services.AddHeaderPropagation(options =>
{
options.Mapping = (context, request) =>
{
foreach (string key in mapping.Keys)
{
var headerValue = context.Request.Headers[key];
if (StringValues.IsNullOrEmpty(headerValue))
{
continue;
}
request.Headers.TryAddWithoutValidation(mapping[key], (string[])headerValue);
}
};
});
} Then in the simple case, a user can do something like the below: var mapping = new NameValueCollection()
{
["Referer"] = "Referer",
["User-Agent"] = "X-Forwarded-User-Agent",
["X-Correlation-Id"] = "X-Correlation-Id",
};
services.AddHeaderPropagation(mapping); But for a more advanced implementation, the user can just bring their own implementation and be advanced as they like using the |
I want to get some real examples of what people use the mapping code for, is |
So internally we map headers so that we can trace things in our Kibana logs so we can see where traffic originated as it goes through the layers of our application, such as from our DDoS edge, to HAProxy, to our web tier, down to our microservices/APIs. In one case we have a header called In another case we map We also have a third case where we pass-through a header if present, but generate a new value if not to forward on. This would be something more advanced that would either still need a custom handler today, or could use something like the advanced delegate-based mapper. |
@martincostello the first two of those are variations of distributed tracing. No? If by default we forwarded a correlation id header would that satisfy most of your requirements for the tracing/identifiers? The distributed tracing spec that I am thinking of us supporting by default also has a concept like baggage from Zipkin, which is an arbitrary mapping of keys to values that will flow as well as the correlationId. If you had those two features would that satisfy all the cases you know of? I have a suspicion that a distributed tracing system is what everyone is using this for and am looking for exceptions to that or limitations that would make it not work :). |
I'm not arguing that we don't do this feature. But if the majority of customer cases are satisfied by correlationIds and perhaps some baggage then we should possibly focus our efforts there first. For example, a scenario that would need mapping outside of the tracing system is testing in production where you flow a header that informs some load balancer to flow your requests to a specific test instance of a microservice. |
Yep, it's definitely similar to/is-kinda distributed tracing, it's just something custom and internal that's been around for years and years in our many applications that everything has to follow by convention. At the moment for our .NET Core stuff we're doing this semi-manually with a hand-coded delegating handler we distribute via ProGet, but something built-in we could just configure would be nicer, however that manifests itself. I think the key "want" for that scenario for us is being able to map one header already present to another name, rather than only a simple as-is propagation. |
I'd like to help moving this forward. Would be creating a PR the best approach? Thanks |
For people who step in this issue looking for a way to easily propagate the ISTIO tracing headers like I did, we created a library just for that, you can find it here: https://github.com/nosinovacao/istio-tracing-aspnetcore . @alefranz it also may be of help for your PR. |
@Symbianx I was actually waiting for a reply to submit a PR but I had already hacked some code. I've now submitted it, IT would be great if you can have a look and let me know if this cover your needs. I will try to have a look at that library tomorrow. |
I’ve been following this as a micro service developer, and really appreciate this is being looked at. My main use case is in terms of tracing and tracking but do also see value in propagating other headsets including any authentication tokens for end-to-end authentication between micros services. It would also be useful to hook any header propagation middleware such that one can leverage the hook to push headers into properties in ones logging context in a central place (like a SeriLog enricher) |
Is #1099 really viable with multithreading? I'm currently using v2.2 with the need to propagate a header. However, I'm stuck with the old ASP.NET Web API 2, so instead of IHttpContextAccessor, I have to make do with HttpContext.Current. Obviously I ran into trouble the moment the HttpClient was being used from another thread (therefore with no associated HttpContext), because the delegating handler's SendAsync() was also running on that thread. I read the IHttpContextAccessor is not thread safe, I'm assuming it has the same limitations whereby you cannot get the HttpContext if the code is not running on the request's thread. Furthermore, the pooling of message handlers is great, but I'm wondering if the design isn't fundamentally flawed. Apparently it does not allow to get scoped information into the handler instance, and that creates all sorts of frustrations. |
This will be included in preview 6. dotnet/aspnetcore#9793 Thanks @alefranz for doing 99% of the work for this! |
I've aslo published an unofficial backport to AspNetCore 2.1 and 2.2. I'll update it now with the latest changes. |
@alefranz Thanks a lot. I understand now, the key to getting this to work across threads and through the asynchronous execution flow is to use AsyncLocal. |
@alefranz Nice! Is there a standardized compatible with this library, to support a way to pick the current header values up, such that they could be injected into say the Serilog context via:
with each scope/request being processed such that any logging pick up those header values (where |
We should consider providing an in-the-box solution for propagating headers from the current request to an outgoing http request.
This would consist of a middleware that can be configured to stamp certain headers on a feature of the current request. Then a message handler uses
IHttpContextAccessor
to grab the feature and apply the headers to outgoing requests.Maybe similar to:
https://github.com/stevejgordon/CorrelationId/tree/dev/src/CorrelationId from @stevejgordon or
(sample @davidfowl made)
Also consider using
Activity
The text was updated successfully, but these errors were encountered: