lmolkova / correlation

Hi there 👋

I'm working on Azure SDK (Azure Monitor and Application Insights in the past) and contribute to OpenTelemetry Semantic Conventions in messaging and HTTP areas.

📚 Check out my new book on modern distributed tracing on .NET and OpenTelemetry or just play with examples! @ or 📧 me what you think.

correlation's Issues

Removal of Filtering logic

I am strongly skeptical of the filtering logic we currently have (most things in CoorelationConfigurationOptions, but in particular EndpointFilter*.

The idea is that in theory this will allow you to AVOID adding outgoing ID/Baggage to particular URL targets. However this is at best a optimization (adding the headers is not that expensive), and it is not clear that filtering by URL pattern is a good idea (who actually knows that particular patterns are OK to not have?). What drove us to use the default patterns that are there? Is it a big deal to simply not have that filter (effectively do it for everything)?

My belief is that other sampling (simply doing one every 100 requests) is far more important than optimizing particular URL targets, and given that the extra complexity of plumbing the URL based filtering is probably not worth it.

Finally one of the ideas is that filtering is a logging function, not a activity function. Thus in particular for System.Net.Http (which will only be instrumented with DiagnosticSource, we should be using DiagnosticSource's filtering logic (It has an 'IsEnabled() check). We should use that and see if that is enough.

What do you think? Is it reasonable to simply rip it out the config for activities?

First round of feedback of the Correlation library

These are more reactions. They may not be completely thought through. However I do like what I see so far.

• Span needs a Finish() API and an end-time.
• Do we need PreciseStartTimeStamp? Why long vs DateTime?
• Baggage needs to be tuned for the functional style.
• Tags should be optimized for the few key's case. (List of key values is fine).
• Should CorrelationID simply be key-value baggage?
• SpanState should probably just be static methods on Span (don't need another class).
• SpanState needs to protect itself against incorrect usage (you should pass yourself to Pop, and that pops everything and including that Span.
• I want to understand the value of IEndPointFilter.
• Should Span() constructor be hidden (start with this see how far we can go).
• Can we instrument Incomming HTTP requests with DiagnosticSource? Is that the way to go?
• I would really like it if this stuff did not depend on ILogger, Instead Ilogger depends in this stuff. After all there are potentally many loggers, but only one correlator, so making the correlator know about loggers seems bad.
• Id creation should be owned by Span (and in particular they should probably not be GUIDS) we should be able to make hierarchical IDs with little problem.

As general advice, we should look over the Java implementation of open tracing, and where it differs from our we should be happy with those differences.

Second Round Of Activity Feedback

I have thought about the problem some and have some very concrete suggestions on various design issues that I will outline here. We should meet on Monday to go over them. I am getting pretty happy with the design I am outlining below. THere is working code in my fork under the Work.1-2 branch

https://github.com/vancem/correlation/tree/Work.1-2

which I have submitted as a pull request in case that is more convenient.

#6

Short term long term: First I will describe the design I want if we can modify everything wherever it lives. There are work-arounds (along the lines of your use of DiagnosticSource to hook System.Net.Http) that I will describe last, but don't let that perturb the better design.

General Architecture: First there is a general mismatch between OpenTracing (which does both logging and correlation, and assumes only one kind of tracing API), and what we need (which is a generic correlation mechanism that can be used for several (three) logging libraries we have. My general resolution for this is

1. Make believe we only are instrumenting library code with DiagnosticSource and design Activities to be first and foremost good for that case. Basically make DiagnosticSource + Activities have the functionality of OpenTracing. Note that this works well because I believe we can keep the coupling low, AND all other loggers can be built 'on top'.

 1A) Thus at places where we do diagnosticSource.Write("XXXStart", args), we also (right before it) start the activity ActivityStart("XXX"), and similarly call ActivityStop("XXX") right after the Write("XXXStop").    I have helpers that do this that should go into DiagnosicSource.   
 2) Since DiagnosticSource is always in-proc, when people need the information they can get it from Activity.Current.   This makes things reasonably decoupled.  
 3) The general dependency is that loggers can depend on Activity but not the other way around.  Thus DiagnosticSource can depend on Activity.  
 4) I think Activity can simply live in the Microsoft.Diagnostic.DiagnosticSource.dll, and the class will live in the System.Diagnostic namespace.   We may wish to rename it 'DiagnosticActivity' but I could live with either (I note that things like Process, and Thread are not called 'DiagnosticProcess' or 'DiagnosticThread', so there is precedent for calling it just 'Activity'.  
 5) What is nice about this is that filtering 'falls out' in the sense that DiagnosticSource has a filtering mechanism and we just piggy-back on that.    Thus when "XXXStart" events are on, you get the activity tracking as well, but otherwise not (thus it is fine-grained).  
 6) Because we generally will be logging a DiagnosticSource event on starts and stops, we may not need the C# events on Activity.   We also don't in theory need start time duration or tags, but I am thinking we will keep those (to encourage uniformity). 

ILogger things will be built completely on TOP of the DiagnosticSource events (thus ILogger depends on DiagnosticSource.dll. My expectation is that we would create two NEW ILogger that will log start and stop events with ILogger at HTTP incomming and Outgoing events, using DiagnosticSource to get the hooks. For incomming HTTP, this code can go into Microsoft.AspNetCore.Http since they already probably have a dependency on ILogger, but for System.Net.Http, it can't go there. It will go into its own DLL probably. (however I really want this to be on by default so I would prefer to 'attach' to a DLL that is already likely to be used in most cases).

As always, this probably brings up more questions than answers. We can talk on Tuesday.

Move RFC proposals to the more official location

Where those will go? I can host them into Application Insights documentation if needed. It's easy to contribute to.

Update correlation proposal v2 in accordance to what was implemented in AI

We are using Request-Context for both - request and response. Let's update the proposal. I'll try to send PR tomorrow.

Also mention scenarios:

Also distributed storage is not the only purpose for the Request/Response context. Beside storage identifier it can carry the:

• Version of API that “parent” request implements. It is not the correlation API version, just a parent’s request.
• Tenant that “parent” request makes work for. This tenant is not necessarily shared across all requests in the trace as backend may be tenant-neutral or batch-process data for multiple tenants
• “Parent” requests route that identifies the request and allows the grouping by it. This name is not necessarily the name you want to propagate two levels down dependencies tree and it only makes sense for immediate child for grouping
• “Parent” request device type like “mobile app v12” or “browser”

Can I use this for netcoreapp1.1?

Hi,

I am looking for adding CorrelationId for every incoming request & outgoing request for my netcoreapp1.1 application?. Will I be able to use this?

My idea was to have only one correlationid for all calls. Some thing like , there are 3 services ServiceA calls ServiceB which calls ServiceC. When a request comes in for ServiceA, it creates a correlationid C1 and when ServiceA calls ServiceB it creates correlationid C2 and the outgoing request will have a header value like C1|C2. When ServiceB calls ServiceC it will create correlationid C3 with the header value as C1|C2|C3. This will help when I search by C1, I can get all the logs related to the parent correlationid.

Is there a way to use like above scenario?