eclipse-hono / hono

Eclipse Hono™ Project
https://eclipse.dev/hono
Eclipse Public License 2.0
451 stars 137 forks source link

Replace OpenTracing instrumentation with OpenTelemetry #3212

Open calohmn opened 2 years ago

calohmn commented 2 years ago

Follow-up of #3190: The OpenTracing instrumentation (already using the OpenTelemetry OpenTracingShim) should be replaced with using OpenTelemetry directly, removing all OpenTracing dependencies.

calohmn commented 2 years ago

I think we should also aim at (EDIT) consider removing many if not all some spanContext parameters passed around in Hono API methods.

The standard way of obtaining the relevant current span context is by means of the OpenTelemetry ContextStorage implementation, by default using a ThreadLocal. Vert.x provides a VertxContextStorage implementation, using the current Vert.x context. This requires making sure that each request being traced has its own Vert.x context in the form of a DuplicatedContext, as described here.

For each of the Hono API requests, where there is always just one current span operation (in a hierarchy of parent spans), the context propagation is straightforward then, requiring only the initial Vert.x DuplicatedContext to be set for the request. Then each (child) span can set itself as the current span

  try(Scope scope = childSpan.makeCurrent()) {
    // do stuff
  } finally {
    childSpan.end();
  }

In some cases however, there are concurrent spans in Hono, like in this example: ConcurrentSpans Both "assert Device Registration" and "get Tenant by ID" are children of "upload telemetry".

I guess in such a case, we could create new Vert.x DuplicatedContexts for each of the concurrent operations. (We could also check whether creation of a child span is necessary or whether it can be replaced by adding log events to the parent span.) EDIT: Creating extra Vert.x DuplicatedContexts here is probably not a good solution. It adds complexity involving the invocation of the concurrent operations (creating the contexts and running the tasks on them) and concerning the assembling of the operation results, having to explicitly run any subsequent operations on the original context again. Just passing the SpanContext as it is currently done is the better solution here.