Skip to content
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

Extend GenAI system to support IBM Watsonx AI and AWS Bedrock #1574

Merged
merged 6 commits into from
Nov 18, 2024

Conversation

gyliu513
Copy link
Member

@gyliu513 gyliu513 commented Nov 13, 2024

Added IBM Watsonx AI and AWS Bedrock to the list of possible values that the gen_ai.system attribute can take.

Changes

The following are added:

  • IBM Watsonx AI
  • AWS Bedrock

Please provide a brief description of the changes here.

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

@gyliu513
Copy link
Member Author

@lmolkova @lzchen can you help review? I want to start the process of contributing watsonx to genai instrumentation, thanks!

Copy link
Contributor

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@lmolkova lmolkova requested a review from a team November 15, 2024 18:30
@lmolkova
Copy link
Contributor

@open-telemetry/semconv-genai-approvers please take a look

Copy link
Member

@drewby drewby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@lmolkova lmolkova enabled auto-merge (squash) November 18, 2024 17:17
@lmolkova lmolkova merged commit 25e0bae into open-telemetry:main Nov 18, 2024
13 of 14 checks passed
@gyliu513 gyliu513 deleted the wx branch November 18, 2024 17:49
@lmolkova
Copy link
Contributor

@gyliu513 we'll need to release the new version of semconvs first. We're due for release, but with KubeCon we got behind and it might take 1-2 weeks.

Then someone (usually me, but can be anyone else) needs to regenerate code in https://github.com/open-telemetry/opentelemetry-python/tree/main/opentelemetry-semantic-conventions.

@mxiamxia
Copy link
Member

mxiamxia commented Dec 12, 2024

Hi @gyliu513 @lmolkova , is there a convention for specifying GenAI attribute values or, more generally, in OpenTelemetry? For instance, _ is used as delimiter in AWS service attribute values, like aws_sqs in Java SDK below. And this proposal uses aws.bedrock. Is it more preferred way to use . delimiter for gen_ai.system attribute specifically?

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/c6a6fa622465ebf931ed5860987cb0ecc6a04a16/instrumentation/aws-lambda/aws-lambda-events-2.2/library/src/main/java/io/opentelemetry/instrumentation/awslambdaevents/v2_2/internal/SqsEventAttributesExtractor.java#L23

@gyliu513
Copy link
Member Author

@mxiamxia I think there is no standard for this yet, you may also see that vertex ai is using vertex_ai. Do you want to change the value to aws_bedrock?

@lmolkova I think it will be great if we can keep consistent for all LLM platforms, like what is the recommended delimiter for the attribute values.

@lmolkova
Copy link
Contributor

. should be used by default. _ should be used to separate words that make no sense without each other - https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attribute-naming.md

Think about it as namespaces in the code. There could be a lot of different things in aws namespace, so aws.bedrock, but it'd be rate_limiting instead of rate.limiting.

It took us a bit to come up with this rule of thumb, so you might see a lot of examples in the repo that don't follow this patter yet (or not worth breaking). Anyway, we try to follow this pattern for everything new.

@mxiamxia
Copy link
Member

Thank you all for the response. AWS Observability team is working on supporting AWS Bedrock in CloudWatch by adhering to the OpenTelemetry LLM Spec. We will follow the aligned pattern and collaborate with you to contribute the relevant changes in OTel SDKs.

mxiamxia pushed a commit to aws-observability/aws-otel-js-instrumentation that referenced this pull request Dec 16, 2024
*Description of changes:*
Updating `gen_ai.system` attribute key to better align with upstream
Otel conventions.

Context:
open-telemetry/semantic-conventions#1574 (comment)

*Test plan:*
Ran updated unit tests and contract tests.
<img width="2560" alt="Screenshot 2024-12-16 at 9 57 36 AM"
src="https://github.com/user-attachments/assets/8e5e7ff5-1608-417a-bc9d-cfb72de538b3"
/>

By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.
mxiamxia added a commit to aws-observability/aws-otel-python-instrumentation that referenced this pull request Dec 16, 2024
*Description of changes:*
Updating `gen_ai.system` attribute key to better align with upstream
Otel conventions.

Context:
open-telemetry/semantic-conventions#1574 (comment)

*Test plan:*
Ran updated unit tests and contract tests.
<img width="2560" alt="Screenshot 2024-12-16 at 10 42 28 AM"
src="https://github.com/user-attachments/assets/b0886292-5a99-4e90-87b0-4ee009a3553e"
/>
<img width="2560" alt="Screenshot 2024-12-16 at 10 58 38 AM"
src="https://github.com/user-attachments/assets/4dab00d0-9141-43f5-980d-fdcd6924d69f"
/>


By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice.

---------

Co-authored-by: Min Xia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants