Support OpenTelemetry

[iancooper]

It needs some design thinking in this thread, but we should decide if it makes sense to support OpenTracing

http://opentracing.io/documentation/pages/instrumentation/instrumenting-frameworks

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[iancooper]

We could probably stab at an initial version using Jaeger

https://github.com/yurishkuro/opentracing-tutorial

[iancooper]

Oh and as it may not be obvious. We would look to implement this as an attribute, similar to the existing Monitoring attribute which you add to a handler to trace handling individual messages. We could choose to instrument parts of the infrastructure too, but it is harder to tie that to tracing the path of a request through a distributed system, over integrating with say Prometheus to provide operations metrics, which may be better.

We might want to use the Control Bus to push metrics that Prometheus could monitor from a a server collating control bus data and pushing to Prometheus, but not the scope of this work.

[holytshirt]

Now call Open Telemerty https://opentelemetry.io/

[iancooper]

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/messaging.md

[iancooper]

Moved this to OpenTelemetry, which shows how long it has been open :-D

[Driedas]

I am interested in this, have taken a look at the source code for an attempt to implement this but I am not sure that the extensibility points needed to do so are in place. First, how should this actually behave (I think): opentelemetry-dotnet is based on System.Diagnostics.Activity so there is no need to take a dependency on that library. I believe that on the sending/publishing end, the current activity id should be appended to all outgoing message headers and on the receiving end an activity should automatically get created, it's parent id set to the id from the message header and started (plus adding a couple of useful activity tags like message id and type, topic...). This activity should be then stopped as the very last step in the pipeline, after all the user defined pipeline steps have been completed. This I think is similar to how the HttpClient works, and Jimmy Bogard's similar extension to NServiceBus. Now, why I think that the necessary extensibility points are missing. The execution pipeline in Brighter is configured using a series of handlers that act as middleware. This is however already too late at the receiving end as at this stage you no longer have access to the raw message to be able to get the activity id out of the headers, and I don't think you would want to pollute the actual request objects with an ActivityId. The only places where you have the message headers in the raw form (as far as I can see) is in the message mapper and the message pump. This logic does not belong to the message mapper, and the message pump does not expose an extension point around the DispatchRequest method. Theoretically this could be achievable by implementing custom message pumps derived from MessagePumpAsync and MessagePumpBlocking and wrapping the call to base.DispatchRequest with an activity start and stop (or using an aspect to wrap the current implementations). Alternatively, if the execution pipeline had access to the message headers (maybe on the Context property?), this would be as simple as adding an additional step into the pipeline. The same issue is present at the sending end, requests are sent via the command processor, and the message in its raw form is accessible only at the individual methods when they call into message mapper.

Did I miss something when looking or should I leave this to the more experienced contributors? The reason I started looking into it in the first place is that this feature would actually be useful to me :-)

[iancooper]

HI @Driedas,

I think that my "guess" and it is that, would be that it ought to go into the context bag, and then be accessible from there, and that we would add an attribute to put middleware in the pipeline that began and stopped the activity (its the Russian doll model so you can wrap later steps). But beyond that, I've not investigated the problem that deeply.

But if we agree that this is a middleware concern, that uses the context bag, we can begin to think about other issues. For example, where do we populate the context bag with this information, which lives on the message header, if the sender supports it. We defer to transports to turn raw bytes into MessageHeader and MessageBody, and then turn that into types in the pipeline. It may make sense to put this in the Header, but where in our pipeline can we hand from header to context etc.

I can have a look and offer an opinion, but also happy to listen to your ideas.

Pulling in @preardon who is thinking about our observabiity over all

[iancooper]

But @Driedas we would certainly welcome working with you on this

[iancooper]

@Driedas How about this?

• We discuss "how" to do this?
• We write up an ADR
• You can then raise a PR against that.

I'm trying to move us to use ADRs so that we don't have to trawl issues

@preardon @holytshirt

[Driedas]

Hi @iancooper sounds great to me! Using the context bag looks like the easiest/least invasive thing to implement on the receiving end. Are you thinking of exposing all of the message headers or just the activity id? Expose the headers via the already existing bag or provide a new one (read only maybe) for headers only?

Happy to help with the implementation once the ADR is settled on...

[preardon]

First release of this is in, I'm sure there will be some changes, however it is currently in a state that it will support Open Telemetry

@iancooper Did you want to keep this one open