-
Notifications
You must be signed in to change notification settings - Fork 126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to code attributes for OpentelemetryPhoenix #432
Switch to code attributes for OpentelemetryPhoenix #432
Conversation
d42f028
to
40d615d
Compare
40d615d
to
9fce9a5
Compare
Hey @danschultzer. Code attributes are handled in open-telemetry/opentelemetry-erlang#808. It's not something for instrumentation libraries to handle or co-opt. It's an API concern and should be in the next release of the API. |
Yeah @bryannaegele, makes sense to use it for the trace caller, but would this work with the Phoenix instrumentation or any instrumentation using telemetry integration? The semconv describes the code attributes as meant to be for the operation that the span describes:
I think this will just pick up the instrumentation module since it's listening to telemetry events and won't describe where the operation happened. |
Yeah, that's a fair point however you have to keep in mind that this is a single span encompassing the adapter and any Phoenix operations, so the code namespaces would appropriately be at the adapter-level. There are not Phoenix-specific spans. If you wanted to consider adding these on new attributes for Phoenix then that may be an option. But the existing attributes must also remain. I do not want to introduce a breaking change. |
Ah yeah, makes sense. The motivation here was to get parity with the plug/function attributes for the LiveView module/function, and figured that really it could all just be code attributes to keep it generic. I can push another PR to just set separate attributes for Phoenix LiveView module and function to see how that looks. |
Stacked on #431 which needs to get in first. Check the latest commit for the changes here.
This conforms closer to the attributes in 1.27 docs:https://github.com/open-telemetry/semantic-conventions/blob/v1.27.0/docs/attributes-registry/code.md
phoenix.plug
andphoenix.action
is actually just the module and function.It does introduce a bunch of breaking changes so let me know if backwards compability is required. I can keep the old attributes, but not sure if we should just go with major release instead since there's a lot of things that are different now? (same question as in #430)
Changes
phoenix.plug
->code.namespace
phoenix.action
->code.function
Adds
code.namespace
to liveview spanscode.function
to liveview spans