-
Notifications
You must be signed in to change notification settings - Fork 358
Added AWS SDK Changes to support Bedrock spans and Application Signals #3257
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
base: main
Are you sure you want to change the base?
Conversation
src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSServiceHelper.cs
Outdated
Show resolved
Hide resolved
src/OpenTelemetry.Instrumentation.AWS/Implementation/AWSServiceHelper.cs
Outdated
Show resolved
Hide resolved
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@normj, @muhammad-othman, @srprash, do you have a plan to review to should we close this PR? |
muhammad-othman
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.
Sorry for the late review, somehow I missed it :(
| <ItemGroup> | ||
| <PackageReference Include="AWSSDK.Core" VersionOverride="[4.0.0, 5.0.0)" /> | ||
| <PackageReference Include="AWSSDK.SimpleNotificationService" VersionOverride="[4.0.0, 5.0.0)" /> | ||
| <PackageReference Include="AWSSDK.BedrockRuntime" VersionOverride="[4.0.0, 5.0.0)" /> |
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.
Why do we need AWSSDK.BedrockRuntime dependency?
Changes
Added changes to detect bedrock spans and populate attributes accordingly. Also added logic to populate aws specific attributes used by AWS Application Signals like "aws.region", "aws.service", "aws.operation", "aws.requestId", etc. We are currently using our own fork of Opentelemetry-Instrumentation-AWS here: https://github.com/aws-observability/aws-otel-dotnet-instrumentation/tree/main/src/OpenTelemetry.Instrumentation.AWS. This change syncs our local fork so that we can stop relying on it and instead reference this directly instead.
I did some manual testing and was able to verify that the spans created contain the required information.
This PR builds on top of this PR: #2458. The old PR went stale especially after moving from AWS SDK v3 to v4. Most of the comments there have been addressed and most of the semconvs used here are now merged in the semconv pkg.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial changes