From 62031eb11315a148242969b612c2b8cd49d7843f Mon Sep 17 00:00:00 2001
From: kruskal <99559985+kruskall@users.noreply.github.com>
Date: Sat, 6 Aug 2022 08:32:42 +0200
Subject: [PATCH] feat: update sns span.name to reflect the current spec

'span.name' is correctly updated to 'SNS PUBLISH to <specific-name>'.

'TargetArn' is parsed and the endpoint UUID is excluded from the
'span.name'.

Add support for 'PhoneNumber' (note that the actual phone number is
not stored.)

See https://github.com/elastic/apm/blob/main/specs/agents/tracing-instrumentation-aws.md#sns-aws-simple-notification-service

Add more test cases to increase coverage.
---
 module/apmawssdkgo/sns.go      | 49 +++++++++++++++++++++++++---------
 module/apmawssdkgo/sns_test.go | 30 +++++++++++++++++++--
 2 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/module/apmawssdkgo/sns.go b/module/apmawssdkgo/sns.go
index 258ef3c5d..397e1e55a 100644
--- a/module/apmawssdkgo/sns.go
+++ b/module/apmawssdkgo/sns.go
@@ -41,7 +41,7 @@ func newSNS(req *request.Request) (*apmSNS, error) {
 
 	topicName := getTopicName(req)
 	if topicName != "" {
-		name += " " + topicName
+		name += " to " + topicName
 		resourceName += "/" + topicName
 	}
 
@@ -74,21 +74,44 @@ func (s *apmSNS) setAdditional(span *apm.Span) {
 }
 
 func getTopicName(req *request.Request) string {
-	// TODO: PhoneNumber is the third possibility, but I'm guessing we
-	// don't want to store that for customers?
-	arn := req.HTTPRequest.FormValue("TopicArn")
-	if arn == "" {
-		arn = req.HTTPRequest.FormValue("TargetArn")
+	// format: arn:aws:sns:us-east-2:123456789012:My-Topic
+	// should return My-Topic
+	if topicArn := req.HTTPRequest.FormValue("TopicArn"); topicArn != "" {
+		idx := strings.LastIndex(topicArn, ":")
+		if idx == -1 {
+			return ""
+		}
+
+		// special check for format: arn:aws:sns:us-east-2:123456789012/MyTopic
+		if slashIdx := strings.LastIndex(topicArn, "/"); slashIdx != -1 {
+			return topicArn[slashIdx+1:]
+		}
+
+		return topicArn[idx+1:]
 	}
 
-	// SNS ARN can be in the following formats:
-	// - arn:aws:sns:us-east-2:123456789012:MyTopic
-	// - arn:aws:sns:us-east-2:123456789012/MyTopic
-	parts := strings.Split(arn, "/")
-	if len(parts) == 1 {
-		parts = strings.Split(arn, ":")
+	// format: arn:aws:sns:us-west-2:123456789012:endpoint/GCM/gcmpushapp/5e3e9847-3183-3f18-a7e8-671c3a57d4b3
+	// should return endpoint/GCM/gcmpushapp
+	if targetArn := req.HTTPRequest.FormValue("TargetArn"); targetArn != "" {
+		idx := strings.LastIndex(targetArn, ":")
+		if idx == -1 {
+			return ""
+		}
+
+		endIdx := strings.LastIndex(targetArn, "/")
+		if endIdx == -1 {
+			return ""
+		}
+
+		return targetArn[idx+1 : endIdx]
 	}
-	return parts[len(parts)-1]
+
+	// The actual phone number MUST NOT be included because it is PII and cardinality is too high.
+	if phoneNumber := req.HTTPRequest.FormValue("PhoneNumber"); phoneNumber != "" {
+		return "[PHONENUMBER]"
+	}
+
+	return ""
 }
 
 // addMessageAttributesSNS adds message attributes to `Publish` RPC calls.
diff --git a/module/apmawssdkgo/sns_test.go b/module/apmawssdkgo/sns_test.go
index 14fb137c4..ee94df392 100644
--- a/module/apmawssdkgo/sns_test.go
+++ b/module/apmawssdkgo/sns_test.go
@@ -44,7 +44,7 @@ func TestSNS(t *testing.T) {
 		ignored, hasTraceContext, hasError bool
 	}{
 		{
-			name:            "SNS PUBLISH myTopic",
+			name:            "SNS PUBLISH to myTopic",
 			action:          "publish",
 			resource:        "sns/myTopic",
 			topicName:       "myTopic",
@@ -58,7 +58,7 @@ func TestSNS(t *testing.T) {
 			},
 		},
 		{
-			name:            "SNS PUBLISH myTopic",
+			name:            "SNS PUBLISH to myTopic",
 			action:          "publish",
 			resource:        "sns/myTopic",
 			topicName:       "myTopic",
@@ -70,6 +70,32 @@ func TestSNS(t *testing.T) {
 				})
 			},
 		},
+		{
+			name:            "SNS PUBLISH to endpoint/GCM/gcmpushapp",
+			action:          "publish",
+			resource:        "sns/endpoint/GCM/gcmpushapp",
+			topicName:       "endpoint/GCM/gcmpushapp",
+			hasTraceContext: true,
+			fn: func(ctx context.Context, svc *sns.SNS) {
+				svc.PublishWithContext(ctx, &sns.PublishInput{
+					Message:   aws.String("my message"),
+					TargetArn: aws.String("arn:aws:sns:us-west-2:123456789012:endpoint/GCM/gcmpushapp/5e3e9847-3183-3f18-a7e8-671c3a57d4b3"),
+				})
+			},
+		},
+		{
+			name:            "SNS PUBLISH to [PHONENUMBER]",
+			action:          "publish",
+			resource:        "sns/[PHONENUMBER]",
+			topicName:       "[PHONENUMBER]",
+			hasTraceContext: true,
+			fn: func(ctx context.Context, svc *sns.SNS) {
+				svc.PublishWithContext(ctx, &sns.PublishInput{
+					Message:     aws.String("my message"),
+					PhoneNumber: aws.String("1"),
+				})
+			},
+		},
 		{
 			ignored: true,
 			fn: func(ctx context.Context, svc *sns.SNS) {