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

Capture the SNS topic ARN under the 'messaging.destination.name' span attribute. #10096

Merged
merged 10 commits into from
Jan 3, 2024

Conversation

tduncan
Copy link
Contributor

@tduncan tduncan commented Dec 18, 2023

Addresses #10030

@tduncan tduncan requested a review from a team December 18, 2023 19:38
Copy link

linux-foundation-easycla bot commented Dec 18, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@laurit
Copy link
Contributor

laurit commented Dec 19, 2023

Should the same change also be made for aws 1 instrumentation? Ideally I think we should change the SNS spans to follow messaging semantic conventions like we did for SQS, though this is probably too much effort and out of scope for this issue.

@tduncan
Copy link
Contributor Author

tduncan commented Dec 19, 2023

@laurit, I'd be happy to update the AWS SDK v1 instrumentation as well though I don't see explicit support for SNS as I do in the v2 instrumentation.

@laurit
Copy link
Contributor

laurit commented Dec 20, 2023

@tduncan I think in aws1 sdk you'd need to add an accessor in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/aws-sdk/aws-sdk-1.11/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/RequestAccess.java that lets you reflectively retrieve the value and create an attribute extractor similar to https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/aws-sdk/aws-sdk-1.11/library/src/main/java/io/opentelemetry/instrumentation/awssdk/v1_11/AwsSdkExperimentalAttributesExtractor.java Can't use AwsSdkExperimentalAttributesExtractor because experimental (non spec) attributes are collected only when enabled with a flag. And add that extractor to

and Hopefully that would be enough (besides aws1 tests also run camel-2.20 tests, these also rely on the aws1 instrumentation)

@tduncan
Copy link
Contributor Author

tduncan commented Dec 21, 2023

@laurit, thanks for the tips! I'll explore the v1 SDK instrumentation in more detail.

@Override
public void onStart(AttributesBuilder attributes, Context parentContext, Request<?> request) {
String destination = findMessageDestination(request.getOriginalRequest());
if (destination != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually we use

public static <T> void internalSet(
AttributesBuilder attributes, AttributeKey<T> key, @Nullable T value) {
if (value != null) {
attributes.put(key, value);
}
}
which does the null check

@trask trask merged commit 1c1ca91 into open-telemetry:main Jan 3, 2024
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants