-
Notifications
You must be signed in to change notification settings - Fork 300
Split multi use gen-ai spans in to 2 distinct spans #2467 #2484
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
Split multi use gen-ai spans in to 2 distinct spans #2467 #2484
Conversation
917c014 to
ad4efe1
Compare
ad4efe1 to
1c1b0f6
Compare
Signed-off-by: James Thompson <[email protected]>
1c1b0f6 to
78941e5
Compare
|
This pr has been split to only include the gen-ai part. @open-telemetry/semconv-genai-approvers can you review |
lmolkova
left a comment
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.
I don't believe that splitting spans by kind improves end user or instrumentation author experience. Now we duplicate huge span tables and the only difference between them is the kind.
Moreover, we know for a fact that client/internal is frequently a preference - e.g. Spring AI uses internal kind for external model calls - spring-projects/spring-ai#1174, more context #1315
|
So i have fixed the issue introduced by rebasing as identified by @gyliu513 and the internal span now no longer has the server attributes with the logic being if you know that you are talking to a sever then you aren't internal. Is there any other attributes which only apply when using a server based gen-ai model? For instance are usage tokens still applicable? @lmolkova i am aware of the ambiguity around span king usage etc and believe that is addressed by using the phrase Should rather than must. I think this scenario is different as the documentation is clearly talking about 2 different kinds of activities, hence splitting them and now removing the non applicable server attributes. If there is others happy to move them to create a bigger difference in definitions. |
23347fe to
11e3420
Compare
|
I still don't understand which problem this PR is trying to solve and how is it solved by introducing |
Progresses #2467
Changes
In a couple of scenarios spans either had 2 name definitions or 2 span kinds. Each of these were for different use cases, hence these have been split into 2 spans.
Reason for the change
This is a blocker for moving to code generated span registries as a span can only have 1 type, hence should be separate definitions.
Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.
Merge requirement checklist
[chore]