open-telemetry / opentelemetry-erlang-contrib

OpenTelemetry instrumentation for Erlang & Elixir
https://opentelemetry.io
Apache License 2.0
155 stars 113 forks source link

Cowboy spans does not has child spans #132

Open juancgalvis opened 1 year ago

juancgalvis commented 1 year ago

Maybe this context should be set on start event, but it was called on stop event.

https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/c8b760e77601a66e20041957d5c102f455fa71d5/instrumentation/opentelemetry_cowboy/src/opentelemetry_cowboy.erl#L53

My spans are shown in two different like this:

image

I want that all my own spans be in the main span HTTP POST.

Thank you for your help or guide on how to achieve it.

tsloughter commented 1 year ago

I'm not sure what you mean. The context is set on the start event.

I'm guessing your issue is related to this happening in a separate process.

@bryannaegele that is it, right?

bryannaegele commented 1 year ago

Yes. Addressed in #133 assuming Phoenix is being used but it kinda looks like this is just a cowboy server. Is that right @juancgalvis? If it's just cowboy, then maybe #104 is relevant

juancgalvis commented 1 year ago

Yes. Addressed in #133 assuming Phoenix is being used but it kinda looks like this is just a cowboy server. Is that right @juancgalvis?

Yes @bryannaegele it's a cowboy with plug, I have already tested with a modified code using plug events and it works, so the problem could be related to #104 because the process that handles the event is different from the process that creates the new span.

bryannaegele commented 1 year ago

Ok. Could probably be fixed by applying the same treatment to https://github.com/opentelemetry-beam/opentelemetry_plug and move it here

ikavgo commented 1 year ago

Is this the same thing we discussed over slack recently? Anyway, I asked @essen and that's what I learnt:

There is a connection process and then there is a request handling process. I previously thought that there is only one :-) and why? because my opencensus propagator was working! And it was working because it was a middleware, i.e. living in request handling process. So how do we pass context from Stream process to Handling process? Cowboy manages everything here so we can't really plug something into supervisors! Let me quite Loic:

You need to put it in the env, then write a middleware that'll write it in the process dictionary. 
Don't think there's anything simpler. Then all handlers will have it stream handler init's 3rd argument:
  Opts#{env := Env}
Pass it forward to cowboy_stream_h and also insert your middleware there (Opts#{middlewares}) before cowboy_router
essen commented 1 year ago

Cowboy 1.x has only one process. Cowboy 2.x has separate processes (due to how HTTP changed in HTTP/2 onward, requests are now handled concurrently).

bryannaegele commented 1 year ago

This is resolved for Phoenix with #144.

For cowboy users, you'll want to use https://hexdocs.pm/opentelemetry_process_propagator/OpentelemetryProcessPropagator.html directly and add this snippet to your stream handler.

ParentCtx = opentelemetry_process_propagator:fetch_parent_ctx(),
otel_ctx:attach(ParentCtx)