feat: add S3 Bucket and Key params in instrumentation (elastic#3274)

Extract the parameters Bucket and Key from the instrumented requests to S3.
Parameters are added into OTel attributes of the span following the OTel semconv spec

Closes: elastic#3150

Author: david-luna

CHANGELOG.asciidoc

@@ -40,6 +40,10 @@ Notes:
 [float]
 ===== Features
 
+* Add `aws.s3.bucket` and `aws.s3.key` attributes for OpenTelemetry in S3 instrumentation.
+  Spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/semantic_conventions/trace/instrumentation/aws-sdk.yml#L435
+  ({issues}3150[#3150]).
+
 [float]
 ===== Bug fixes
 

lib/instrumentation/generic-span.js

@@ -246,13 +246,18 @@ GenericSpan.prototype._serializeOTel = function (payload) {
     payload.otel = {
       span_kind: this._otelKind
     }
-    if (this._otelAttributes) {
-      // Though the spec allows it ("MAY"), we are opting *not* to report OTel
-      // span attributes as labels for older (<7.16) versions of APM server.
-      // This is to avoid the added complexity of guarding allowed attribute
-      // value types to those supported by the APM server intake API.
-      payload.otel.attributes = this._otelAttributes
+  }
+
+  if (this._otelAttributes) {
+    // Though the spec allows it ("MAY"), we are opting *not* to report OTel
+    // span attributes as labels for older (<7.16) versions of APM server.
+    // This is to avoid the added complexity of guarding allowed attribute
+    // value types to those supported by the APM server intake API.
+    if (!payload.otel) {
+      payload.otel = {}
     }
+
+    payload.otel.attributes = this._otelAttributes
   }
 }
 

lib/instrumentation/modules/aws-sdk/s3.js

@@ -52,8 +52,10 @@ function resourceFromBucket (bucket) {
 // @param {AWS.Request} request https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/Request.html
 function instrumentationS3 (orig, origArguments, request, AWS, agent, { version, enabled }) {
   const opName = opNameFromOperation(request.operation)
+  const params = request.params
+  const bucket = params && params.Bucket
+  const resource = resourceFromBucket(bucket)
   let name = 'S3 ' + opName
-  const resource = resourceFromBucket(request.params && request.params.Bucket)
   if (resource) {
     name += ' ' + resource
   }
@@ -65,6 +67,19 @@ function instrumentationS3 (orig, origArguments, request, AWS, agent, { version,
     return orig.apply(request, origArguments)
   }
 
+  // As for now OTel spec defines attributes for operations that require a Bucket
+  // if that changes we should review this guard
+  // https://github.com/open-telemetry/opentelemetry-specification/blob/v1.20.0/semantic_conventions/trace/instrumentation/aws-sdk.yml#L435
+  if (bucket) {
+    const otelAttrs = span._getOTelAttributes()
+
+    otelAttrs['aws.s3.bucket'] = bucket
+
+    if (params.Key) {
+      otelAttrs['aws.s3.key'] = params.Key
+    }
+  }
+
   const onComplete = function (response) {
     // `response` is an AWS.Response
     // https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/Response.html

test/instrumentation/modules/aws-sdk/s3.test.js

@@ -141,6 +141,9 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 200, response: { encoded_body_size: 177 } }
           },
+          otel: {
+            attributes: { 'aws.s3.bucket': 'elasticapmtest-bucket-1' }
+          },
           outcome: 'success'
         }, 'createTheBucketIfNecessary produced expected span')
 
@@ -159,6 +162,9 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 200 }
           },
+          otel: {
+            attributes: { 'aws.s3.bucket': 'elasticapmtest-bucket-1' }
+          },
           outcome: 'success'
         }, 'waitForBucketToExist produced expected span')
 
@@ -177,6 +183,12 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 200 }
           },
+          otel: {
+            attributes: {
+              'aws.s3.bucket': 'elasticapmtest-bucket-1',
+              'aws.s3.key': 'aDir/aFile.txt'
+            }
+          },
           outcome: 'success'
         }, 'createObj produced expected span')
 
@@ -195,6 +207,12 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 200 }
           },
+          otel: {
+            attributes: {
+              'aws.s3.bucket': 'elasticapmtest-bucket-1',
+              'aws.s3.key': 'aDir/aFile.txt'
+            }
+          },
           outcome: 'success'
         }, 'waitForObjectToExist produced expected span')
 
@@ -213,6 +231,12 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 200, response: { encoded_body_size: 8 } }
           },
+          otel: {
+            attributes: {
+              'aws.s3.bucket': 'elasticapmtest-bucket-1',
+              'aws.s3.key': 'aDir/aFile.txt'
+            }
+          },
           outcome: 'success'
         }, 'getObj produced expected span')
 
@@ -231,6 +255,12 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 304 }
           },
+          otel: {
+            attributes: {
+              'aws.s3.bucket': 'elasticapmtest-bucket-1',
+              'aws.s3.key': 'aDir/aFile.txt'
+            }
+          },
           outcome: 'success'
         }, 'getObjConditionalGet produced expected span')
 
@@ -249,6 +279,12 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 200, response: { encoded_body_size: 8 } }
           },
+          otel: {
+            attributes: {
+              'aws.s3.bucket': 'elasticapmtest-bucket-1',
+              'aws.s3.key': 'aDir/aFile.txt'
+            }
+          },
           outcome: 'success'
         }, 'getObjUsingPromise produced expected span')
 
@@ -268,6 +304,12 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 404, response: { encoded_body_size: 207 } }
           },
+          otel: {
+            attributes: {
+              'aws.s3.bucket': 'elasticapmtest-bucket-1',
+              'aws.s3.key': 'aDir/aFile.txt-does-not-exist'
+            }
+          },
           outcome: 'failure'
         }, 'getObjNonExistantObject produced expected span')
         t.equal(errors.length, 1, 'got 1 error')
@@ -290,6 +332,12 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 204 }
           },
+          otel: {
+            attributes: {
+              'aws.s3.bucket': 'elasticapmtest-bucket-1',
+              'aws.s3.key': 'aDir/aFile.txt'
+            }
+          },
           outcome: 'success'
         }, 'deleteTheObj produced expected span')
 
@@ -308,6 +356,9 @@ tape.test('simple S3 usage scenario', function (t) {
             },
             http: { status_code: 204 }
           },
+          otel: {
+            attributes: { 'aws.s3.bucket': 'elasticapmtest-bucket-1' }
+          },
           outcome: 'success'
         }, 'deleteTheBucketIfCreatedIt produced expected span')