From de889ebe0c9b7326d89d26f87162aa656d4a6cf3 Mon Sep 17 00:00:00 2001
From: Anusha Lingamneni <anusha.lingamneni@appdynamics.com>
Date: Wed, 11 Oct 2023 11:58:02 -0700
Subject: [PATCH] Capture full ARN for SNS topics in messaging.destination

Problem:
The use of topic names instead of ARNs in `messaging.destination` omits
essential account information, making topic localization cumbersome.

Solution:
Adopt full ARNs as the value for `messaging.destination`, preserving both
topic name and account details, aligning with current SNS ServiceExtension
approach.

Closes #1716
---
 .../src/services/sns.ts                       | 21 +------------------
 .../test/sns.test.ts                          | 14 +++++++------
 2 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-aws-sdk/src/services/sns.ts b/plugins/node/opentelemetry-instrumentation-aws-sdk/src/services/sns.ts
index 9652719e83..5066df16df 100644
--- a/plugins/node/opentelemetry-instrumentation-aws-sdk/src/services/sns.ts
+++ b/plugins/node/opentelemetry-instrumentation-aws-sdk/src/services/sns.ts
@@ -41,7 +41,7 @@ export class SnsServiceExtension implements ServiceExtension {
         MessagingDestinationKindValues.TOPIC;
       const { TopicArn, TargetArn, PhoneNumber } = request.commandInput;
       spanAttributes[SemanticAttributes.MESSAGING_DESTINATION] =
-        this.extractDestinationName(TopicArn, TargetArn, PhoneNumber);
+        TopicArn || TargetArn || PhoneNumber || 'unknown';
 
       spanName = `${
         PhoneNumber
@@ -76,23 +76,4 @@ export class SnsServiceExtension implements ServiceExtension {
     tracer: Tracer,
     config: AwsSdkInstrumentationConfig
   ): void {}
-
-  extractDestinationName(
-    topicArn: string,
-    targetArn: string,
-    phoneNumber: string
-  ): string {
-    if (topicArn || targetArn) {
-      const arn = topicArn ?? targetArn;
-      try {
-        return arn.substr(arn.lastIndexOf(':') + 1);
-      } catch (err) {
-        return arn;
-      }
-    } else if (phoneNumber) {
-      return phoneNumber;
-    } else {
-      return 'unknown';
-    }
-  }
 }
diff --git a/plugins/node/opentelemetry-instrumentation-aws-sdk/test/sns.test.ts b/plugins/node/opentelemetry-instrumentation-aws-sdk/test/sns.test.ts
index ce6ce9c885..e4043e9d45 100644
--- a/plugins/node/opentelemetry-instrumentation-aws-sdk/test/sns.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-aws-sdk/test/sns.test.ts
@@ -73,7 +73,7 @@ describe('SNS - v2', () => {
         .promise();
 
       const publishSpans = getTestSpans().filter(
-        (s: ReadableSpan) => s.name === `${topicName} send`
+        (s: ReadableSpan) => s.name === `${fakeARN} send`
       );
       expect(publishSpans.length).toBe(1);
 
@@ -83,7 +83,7 @@ describe('SNS - v2', () => {
       ).toBe(MessagingDestinationKindValues.TOPIC);
       expect(
         publishSpan.attributes[SemanticAttributes.MESSAGING_DESTINATION]
-      ).toBe(topicName);
+      ).toBe(fakeARN);
       expect(publishSpan.attributes[SemanticAttributes.RPC_METHOD]).toBe(
         'Publish'
       );
@@ -128,7 +128,7 @@ describe('SNS - v2', () => {
         .promise();
 
       const publishSpans = getTestSpans().filter(
-        (s: ReadableSpan) => s.name === `${topicName} send`
+        (s: ReadableSpan) => s.name === `${fakeARN} send`
       );
       expect(publishSpans.length).toBe(1);
       expect(
@@ -186,13 +186,15 @@ describe('SNS - v3', () => {
   describe('publish', () => {
     it('topic arn', async () => {
       const topicV3Name = 'dummy-sns-v3-topic';
+      const topicV3ARN = `arn:aws:sns:us-east-1:000000000:${topicV3Name}`;
+
       await sns.publish({
         Message: 'sns message',
-        TopicArn: `arn:aws:sns:us-east-1:000000000:${topicV3Name}`,
+        TopicArn: topicV3ARN,
       });
 
       const publishSpans = getTestSpans().filter(
-        (s: ReadableSpan) => s.name === `${topicV3Name} send`
+        (s: ReadableSpan) => s.name === `${topicV3ARN} send`
       );
       expect(publishSpans.length).toBe(1);
 
@@ -202,7 +204,7 @@ describe('SNS - v3', () => {
       ).toBe(MessagingDestinationKindValues.TOPIC);
       expect(
         publishSpan.attributes[SemanticAttributes.MESSAGING_DESTINATION]
-      ).toBe(topicV3Name);
+      ).toBe(topicV3ARN);
       expect(publishSpan.attributes[SemanticAttributes.RPC_METHOD]).toBe(
         'Publish'
       );