Update span names for OpentelemetryPhoenix

[danschultzer]

Stacked on #431 which needs to get in first. Check the latest commit for the changes here.

I don't see any convention for how span names should be handled for websocket events other than maybe using the messaging namespace, but I think we want the structure to be more in line with how span names usually look: {operation} {resource}

So instead of MyAppWeb.ResourceLive.Show.mount it becomes live_view.mount /resources/:resource_id (for events it'll be live_view.handle_event /resources/:resource_id hello.

This is a breaking change.

Changes

• Span names changed from {live_view_module}.{handler_function} to live_view.#{handler_function} {route}
• Span names changed from {live_view_module}.handle_event {event} to live_view.handle_event {route} {event}

[tsloughter]

I think this is a breaking change, right? To do that we need really good reason, like there being an Otel spec or semantic convention saying this is how to name them.

Do you know of other implementations in other languages and what they use for such span names? Maybe based on that I could poke around about a semconv proposal to standardize this.

[danschultzer]

Right, this is a breaking change, I've updated the issue to mention this thanks!

This is LiveView specific though so there is no direct spec we can use for this, but this does conform to the general tendency in SemConv using the {operation} {resource} format as the following shows:

https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/messaging/messaging-spans.md#span-name
https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/http/http-spans.md#name
https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/graphql/graphql-spans.md
https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/database/database-spans.md#name

There are alternatives though like:

https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/rpc/rpc-spans.md#span-name

Considering that LiveView is built on top of the Phoenix.Socket messaging system I think it is more correct to following the messaging span format. Or HTTP format as there is a route that is matched in the Phoenix router.

[danschultzer]

And to expand on why adding the route to the span name; I'm adding the route template to the span name as it works better than just the live module. The live route is part of the Phoenix router, and you could use the same live module for multiple routes same way you can use the same controller for multiple routes. The entry point is actually the route rather than the live module (kinda, the devil is in the details).

[bryannaegele]

This is not a review but a general comment on this and a couple of the other PRs that are introducing dependencies on Phoenix, LiveView, etc. Our instrumentation libraries cannot include dependencies on the library being instrumented. There are a few reasons for this but as you come up with ideas or proposals just know that is a hard constraint.

[danschultzer]

Our instrumentation libraries cannot include dependencies on the library being instrumented. There are a few reasons for this but as you come up with ideas or proposals just know that is a hard constraint.

I assume you are talking about #439 that introduces it as a dep for the lib as this one only requires it for dev/test? (from #431 this is just stacked on top of that)

As for #439 I would like to understand the reasons, is there some issue/doc to refer to?

There are some of the existing instrumentation libraries that have direct dependencies currently:

https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/instrumentation/opentelemetry_oban/mix.exs#L47
https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/instrumentation/opentelemetry_req/mix.exs#L66
https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/instrumentation/opentelemetry_tesla/mix.exs#L61

And some libraries have other dependencies that are more loosely related:

https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/instrumentation/opentelemetry_cowboy/rebar.config#L3
https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/instrumentation/opentelemetry_bandit/mix.exs#L78

For #439 I can maybe find a workaround, though I don't know if the compiler resolution is going to make it difficult, and it'll also loose the compiler checks that can report issues early on. But it could maybe be a utility library instead, or maybe more appropriate a propagator library e.g. opentelemetry_phoenix_live_view_propagator.