search

stevejgordon / CorrelationId

An ASP.NET Core middleware component which synchronises a correlation ID for cross API request logging.
MIT License
558 stars 106 forks source link

Consider adding User-Agent #21

Closed RehanSaeed closed 4 years ago

RehanSaeed commented 6 years ago

In my own home grown library I'm handling X-Request-ID but also User-Agent, so I know which app is making a call to my API. I also reflect the User-Agent back to the app much like we both do with the request ID but I don't really need to do that.

I'm plugging both X-Request-ID and User-Agent into Serilog, so all of my logs have these properties.

stevejgordon commented 6 years ago

Hi @RehanSaeed! I've been considering extra properties with the library. I had in mind something similar in terms of a "CallerIdentifier". I'm currently trying to decide how I'd prefer to do that. My thought at this point is maybe allowing a header with keys and values which can then be held on the context. This has it's pros and cons though.

I'll mark this under consideration as I'd like to do something around this in v3.0.

RehanSaeed commented 6 years ago

I was thinking that User-Agent is common enough to warrant a property, as opposed to a dictionary entry.

A dictionary might be a good idea, what other HTTP headers do you envisage you'd want to add to the context? X-Tenant-ID for multi-tenant apps might be an example.

stevejgordon commented 6 years ago

Yeah I can imagine cases where business might want to pass more data along to downstream systems, as you say tenant id is an example. I was also thinking about thinks like a version number as well.

User-Agent might warrant it's own option for ease of use.

RehanSaeed commented 6 years ago

BTW User-Agent can also contain an app version number.

stevejgordon commented 4 years ago

@RehanSaeed - Finally getting around to some work on 3.0.0 features. I've kinda forgotten where my head was at with this issue. On reflection, I'm not entirely sure it's that useful to add since you can grab the UA from the HttpContext if needed.

RehanSaeed commented 4 years ago

Currently investigating open telemetry. I think there is a lot of overlap with this project and some of it has already been implemented in ASP.NET Core. See https://devblogs.microsoft.com/aspnet/improvements-in-net-core-3-0-for-troubleshooting-and-monitoring-distributed-apps/.

stevejgordon commented 4 years ago

Yep agree that's better suited to more complex distributed tracing. I'll close this off and limit this library to a small feature set for those still wanting something simple for just passing correlation IDs around.