Skip to content

CAMEL-15260 - Adds tracing strategy to enable spans for each processor#4001

Merged
oscerd merged 15 commits into
apache:masterfrom
jam01:opentracing-refactor
Jul 20, 2020
Merged

CAMEL-15260 - Adds tracing strategy to enable spans for each processor#4001
oscerd merged 15 commits into
apache:masterfrom
jam01:opentracing-refactor

Conversation

@jam01
Copy link
Copy Markdown
Contributor

@jam01 jam01 commented Jul 15, 2020

This is a considerable refactor and change in how OpenTracing works with Camel.

First, to address CAMEL-15260 I added an InterceptStrategy that will:

  • Create Spans for each processor using ids and component names. This was based on the AWS Xray component
  • Opens/closes a Scope around each processor.

Having opened Scopes within each processor enables two big features:

  • Allows any instrumentation library from opentracing-contrib to pick up the active Spans and use it to create child ones. This should work with or without java agent instrumentation.
  • Gives the user direct access to the active Span for any custom functionality

Second, it changes the the behavior of the OpenTracingTracer to only create one Server Span. Conceptually there should only be a single Server Span while in-process in a single service.

@mcrmfc , this is the approach I was thinking should be implemented. Would this work for your use case?

@mcrmfc
Copy link
Copy Markdown

mcrmfc commented Jul 16, 2020

@jam01 - nice work, think this should more than meet my requirement which was simply to roll spans created outside of Camel's OpenTracingTracer - so yep will close up my PR and leave this one to the maintainers.

@jam01
Copy link
Copy Markdown
Contributor Author

jam01 commented Jul 17, 2020

So I thiiink this is about ready for primetime. @oscerd let me know if it looks good to you whenever you get the chance. No hurry :)

There is one tradeoff decision I made that may be worth putting out for discussion in case you have reservations about it. When creating new Spans for each processor, I decided to use the processor's ID as the operation's name. The alternative was the processor's label which may have more useful (?) information, but I think the critical characteristic is that ID is configurable when building the route. So it allows the user to give each processor a user friendly ID that'd show up in Jaeger or whatever tracing backend is used, instead of a label that might include expressions possibly leaking some sensitive data without being given a choice.

jam01 and others added 13 commits July 17, 2020 00:39
refactored to not create multiple client-server spans
adds a tracing strategy that adds spans for each processor and opens
scopes between processors to enable other instrumentation
Reintroduces the event notifier but skips creating client spans for
internal endpoints
moves members to original order
Uses ActiveSpanManager to keep track of active processor spans. This
allows to create trees of processors and other span creation to be made
under the active processor directly and not the route.
The Java agent no longer use byteman rules
@oscerd
Copy link
Copy Markdown
Contributor

oscerd commented Jul 17, 2020

LGTM. It would be nice if you could add an example of usage here: https://github.com/apache/camel-examples

@oscerd
Copy link
Copy Markdown
Contributor

oscerd commented Jul 17, 2020

I mean once merged. I'll merge it today I believe.

@oscerd oscerd merged commit d556723 into apache:master Jul 20, 2020
@oscerd
Copy link
Copy Markdown
Contributor

oscerd commented Jul 20, 2020

Thanks a lot.

@valdar
Copy link
Copy Markdown
Member

valdar commented Jul 23, 2020

@jam01 I am wrapping my head around this and I would really appreciate some hints on how is an external trace coming in the form of a string header "uber-trace-id" handled?

As an example think about a camel route starting from an http endpoint an external client calling that endpoint passing an http header "uber-trace-id"=traceId:spanId:parentSpanId:flags. Would that be taken in account so that the camel produced spans are linked to the incoming traceId by the new way of handling the tracing? if yes how?

In other and shorter words: where is this piece of functionality https://github.com/apache/camel/blob/camel-3.4.0/components/camel-opentracing/src/main/java/org/apache/camel/opentracing/OpenTracingTracer.java#L325 handled now?

Thanks in advance!

@jam01
Copy link
Copy Markdown
Contributor Author

jam01 commented Jul 24, 2020

Hey @valdar. There are few things to consider. I'll try to find a bit of time a create a quick write up... There's no automatic way to do that yet. I'm hoping to find some time to address that soon. But I'm pretty sure it's gonna require Java agent instrumentation...

@valdar
Copy link
Copy Markdown
Member

valdar commented Jul 24, 2020

Hey @jam01. I understand there is no automatic way, but what the code at https://github.com/apache/camel/blob/camel-3.4.0/components/camel-opentracing/src/main/java/org/apache/camel/opentracing/OpenTracingTracer.java?rgh-link-date=2020-07-23T08%3A52%3A19Z#L325 was trying to do was exactly that. If I understand it correctly it can be phrased in words as: When an exchange is started to being routed in a camel route, if that exchange contains a header named uber-trace-id use its value to construct a tracing context for the next span.

It might not cover all the cases, it might not be automatic, but from what I remember from using camel-tracing, it was working for the most commonly used components: http, jms, exposing rest to name a few.

Am I missing something obvious that prevents using the same logic?

@jam01
Copy link
Copy Markdown
Contributor Author

jam01 commented Jul 24, 2020

You're absolutely right! So that specific change was meant to accommodate a scenario where when using javaagent instrumentation you let the HTTP/JMS/etc library specific instrumentation create and decorate spans, possibly in a more deterministic manner than camel generic ones can (edit: or for protocols that camel doesn't have decorators for). It seems there was no coverage for a scenario where you want camel to still give it a shot looking in the incoming headers. I missed that!

That said, we have to think about some conditionals.

  • Camel should only attempt to read from the incoming headers when there's not already an active span for that exchange. This is in case instrumentation already extracted the parent from the headers. Activating the span for the exchange in instrumentation is the part I'm still working on.
  • Only read headers for components that are not internal (seda, direct, vm, etc).
  • Only add the Server tag when.... the component is not internal and no span is active for the exchange. Whether we found a parent in the headers or not.

I think this should cover it... I'll add some tests and confirm. Thanks for bringing this up, sorry if this set you back some! Expect a PR soonish...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants