-
Notifications
You must be signed in to change notification settings - Fork 182
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
Convert gen_ai.operation.name
to enum and use it on spans
#1202
Convert gen_ai.operation.name
to enum and use it on spans
#1202
Conversation
fe41b76
to
780a0eb
Compare
/cc @open-telemetry/semconv-llm-approvers |
d9c9f3a
to
8786542
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
drive by feedback, the description of this PR, while significantly changing a fundamental field (span name) does cover "what" like mechanics, but not "why" Can we here and in the future put the motivation for renovations such as these, especially when doing things like enums we expect to break later? It will help be able to jump into a PR and not just assess, yeah.. this changes the span name, but also understand why we are considering it will drift all instrumenters. |
Just wanted to add a note from Langtrace's point of view, regarding the convention for span names, it’s a bit tricky to make the change in our case for us since our SDK provides an option to disable tracing certain methods if the user wants more control and as a result we set span name as the method name. We will have to make some changes to the "disable tracing logic" if we update this - so treading a bit cautiously here. Will make this change pretty soon. |
Replaces #995
Changes
gen_ai.operation.name
to enum. Clarifies name for thechat
operation.gen_ai.system
Merge requirement checklist
[chore]