eclipse-vertx / vertx-tracing

Vertx integration with tracing libraries
Other
42 stars 34 forks source link

Can't get OpenTelemetry Context in different thread #76

Open semistone opened 5 months ago

semistone commented 5 months ago

Questions

I am using vertx webclient in different thread with attach Context

VertxContextStorageProvider.VertxContextStorage.INSTANCE.attach(context);

when PREPARE_REQUEST and CREATE_REQUEST phase, it's still in original thread and still could get Context by

Context.current() 

at this phase, Vertx Context still doesn't exist, so it get context current from OpenTelemetry Context

but in SEND_REQUEST phase, it switch to different thread after create connection. and in this phase will create Vertx Context by vertx.getOrCreateContext() then call

OpenTelemetryTracer.sendRequest

it will try to get OpenTelemetry Context by

VertxContextStorage.INSTANCE.current();

and at this moment, Vertx Context is empty , so it can't get OpenTelemetry Context

currently, I wrote a Handler to propagator OpenTelemetry Context to SEND_REQUEST phase, but not sure is there any other better solution?

Version

4.5.7

Context

Do you have a reproducer?

I changed unit test in vertx-opentelemetry/src/test/java/io/vertx/tracing/opentelemetry/OpenTelemetryIntegrationTest.java

Steps to reproduce

  1. run client in new thread
  2. attach Context to current thread
  3. call vertx webclient request

Extra

tsegismont commented 5 months ago

currently, I wrote a Handler to propagator OpenTelemetry Context to SEND_REQUEST phase, but not sure is there any other better solution?

Can you paste here a snippet with your solution?

semistone commented 5 months ago

currently, I wrote a Handler to propagator OpenTelemetry Context to SEND_REQUEST phase, but not sure is there any other better solution?

Can you paste here a snippet with your solution?

just put Context in httpContext and get it in SEND_REQUEST phase

c.makeCurrent() will save context into VertxContextStorage


    @Override
    public void handle(HttpContext<T> httpContext) {
        try {
            if (httpContext.phase() == ClientPhase.PREPARE_REQUEST) {
                httpContext.set(OPEN_TELEMETRY_CONTEXT, Context.current());
            }
            // send phase need to use PREPARE_REQUEST phase's context
            if (httpContext.phase() == ClientPhase.SEND_REQUEST
                    && httpContext.get(OPEN_TELEMETRY_CONTEXT) instanceof Context c && c != Context.current()) {
                httpContext.set(OPEN_TELEMETRY_SCOPE, c.makeCurrent());
            }
            if ((httpContext.phase() == ClientPhase.RECEIVE_RESPONSE || httpContext.phase() == ClientPhase.FAILURE)
                    && httpContext.get(OPEN_TELEMETRY_SCOPE) instanceof Scope s) {
                s.close();
            }
            httpContext.next();
        } catch (Exception e) {
            log.warn("switch OpenTelemetry Context fail", e);
        }
    }
tsegismont commented 5 months ago

@semistone I can't think of anything much better than to be honest.

I wonder if you have tried OTel instrumentations like this one: https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/vertx/vertx-http-client/vertx-http-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/client

semistone commented 5 months ago

@semistone I can't think of anything much better than to be honest.

I wonder if you have tried OTel instrumentations like this one: https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/vertx/vertx-http-client/vertx-http-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/client

I will try it later, but I guess those instrument will still rely on those Context to pass successfully.

semistone commented 5 months ago

@semistone I can't think of anything much better than to be honest. I wonder if you have tried OTel instrumentations like this one: https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/vertx/vertx-http-client/vertx-http-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/client

I will try it later, but I guess those instrument will still rely on those Context to pass successfully.

I tested and it work

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/vertx/vertx-http-client/vertx-http-client-4.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/vertx/v4_0/client/HandlerWrapper.java

tsegismont commented 4 months ago

Thanks for sharing this information with us @semistone !

I'm closing this since we have a good workaround (thanks to you) documented here and we know using the otel instrumentation solves the problem.