feat(instrumentation-aws-sdk): support migration to stable HTTP semconv (#3077)

Author: trentm

packages/instrumentation-aws-sdk/README.md

@@ -102,13 +102,16 @@ The instrumentation is doing best effort to support the trace specification of O
 
 ## Semantic Conventions
 
-This package uses `@opentelemetry/semantic-conventions` version `1.22+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md)
+This package emits telemetry using a mix of Semantic Convention versions. While not ideal, this is a result of there being a number of separate AWS services instrumented with independent maintenance.
 
-Attributes collected:
+- As a baseline, initial implementations follow [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md).
+- SQS instrumentation was [updated](https://github.com/open-telemetry/opentelemetry-js-contrib/pull/2345) to follow v1.36.0.
+
+Attributes collected (this list is currently not exhaustive):
 
 | Attribute                                     | Short Description                                                                              | Service  |
 | --------------------------------------------- | ---------------------------------------------------------------------------------------------- | -------- |
-| `http.status_code`                            | (aws-sdk) HTTP response status code.                                                           |          |
+| `http.status_code` / `http.response.status_code` | (aws-sdk) HTTP response status code. See "HTTP Semantic Convention migration" note below.   |          |
 | `rpc.method`                                  | The name of the (logical) method being called.                                                 |          |
 | `rpc.service`                                 | The full (logical) name of the service being called.                                           |          |
 | `rpc.system`                                  | A string identifying the remoting system.                                                      |          |
@@ -149,6 +152,31 @@ Attributes collected:
 | `messaging.message_id`                        | A value used by the messaging system as an identifier for the message.                         | sqs      |
 | `messaging.url`                               | The connection string.                                                                         | sqs      |
 
+### HTTP Semantic Convention migration
+
+HTTP semantic conventions (semconv) were stabilized in v1.23.0, and a [migration process](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/http-migration.md#http-semantic-convention-stability-migration)
+was defined.  `instrumentation-aws-sdk` adds some minimal HTTP-related
+attributes on created spans. Starting with `instrumentation-aws-sdk` version
+0.61.0, the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable can be used to
+customize which HTTP semantic conventions are used for those HTTP-related
+attributes.
+
+To select which semconv version(s) is emitted from this instrumentation, use the
+`OTEL_SEMCONV_STABILITY_OPT_IN` environment variable.
+
+- `http`: emit the new (stable) v1.23.0+ semantics
+- `http/dup`: emit **both** the old v1.7.0 and the new (stable) v1.23.0+ semantics
+- By default, if `OTEL_SEMCONV_STABILITY_OPT_IN` includes neither of the above tokens, the old v1.7.0 semconv is used.
+
+For this instrumentation, the only impacted attributes are as follows:
+
+| v1.7.0 semconv     | v1.23.0 semconv             | Short Description         |
+| ------------------ | --------------------------- | ------------------------- |
+| `http.status_code` | `http.response.status_code` | HTTP response status code |
+
+See the [HTTP semconv migration plan for OpenTelemetry JS instrumentations](https://github.com/open-telemetry/opentelemetry-js/issues/5646) for more details.
+
+
 ## Useful links
 
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>

packages/instrumentation-aws-sdk/src/aws-sdk.ts

@@ -40,6 +40,8 @@ import {
   InstrumentationNodeModuleFile,
   isWrapped,
   safeExecuteInTheMiddle,
+  SemconvStability,
+  semconvStabilityFromStr,
 } from '@opentelemetry/instrumentation';
 import type {
   MiddlewareStack,
@@ -57,6 +59,7 @@ import {
 import { propwrap } from './propwrap';
 import { RequestMetadata } from './services/ServiceExtension';
 import { ATTR_HTTP_STATUS_CODE } from './semconv';
+import { ATTR_HTTP_RESPONSE_STATUS_CODE } from '@opentelemetry/semantic-conventions';
 
 const V3_CLIENT_CONFIG_KEY = Symbol(
   'opentelemetry.instrumentation.aws-sdk.client.config'
@@ -70,8 +73,14 @@ export class AwsInstrumentation extends InstrumentationBase<AwsSdkInstrumentatio
   // need declare since initialized in callbacks from super constructor
   private declare servicesExtensions: ServicesExtensions;
 
+  private _semconvStability: SemconvStability;
+
   constructor(config: AwsSdkInstrumentationConfig = {}) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
+    this._semconvStability = semconvStabilityFromStr(
+      'http',
+      process.env.OTEL_SEMCONV_STABILITY_OPT_IN
+    );
   }
 
   protected init(): InstrumentationModuleDefinition[] {
@@ -406,7 +415,15 @@ export class AwsInstrumentation extends InstrumentationBase<AwsSdkInstrumentatio
                   const httpStatusCode =
                     response.output?.$metadata?.httpStatusCode;
                   if (httpStatusCode) {
-                    span.setAttribute(ATTR_HTTP_STATUS_CODE, httpStatusCode);
+                    if (self._semconvStability & SemconvStability.OLD) {
+                      span.setAttribute(ATTR_HTTP_STATUS_CODE, httpStatusCode);
+                    }
+                    if (self._semconvStability & SemconvStability.STABLE) {
+                      span.setAttribute(
+                        ATTR_HTTP_RESPONSE_STATUS_CODE,
+                        httpStatusCode
+                      );
+                    }
                   }
 
                   const extendedRequestId =
@@ -445,7 +462,15 @@ export class AwsInstrumentation extends InstrumentationBase<AwsSdkInstrumentatio
 
                   const httpStatusCode = err?.$metadata?.httpStatusCode;
                   if (httpStatusCode) {
-                    span.setAttribute(ATTR_HTTP_STATUS_CODE, httpStatusCode);
+                    if (self._semconvStability & SemconvStability.OLD) {
+                      span.setAttribute(ATTR_HTTP_STATUS_CODE, httpStatusCode);
+                    }
+                    if (self._semconvStability & SemconvStability.STABLE) {
+                      span.setAttribute(
+                        ATTR_HTTP_RESPONSE_STATUS_CODE,
+                        httpStatusCode
+                      );
+                    }
                   }
 
                   const extendedRequestId = err?.extendedRequestId;

packages/instrumentation-aws-sdk/test/aws-sdk-v3-s3.test.ts

@@ -45,6 +45,7 @@ import { AttributeNames } from '../src/enums';
 import { expect } from 'expect';
 import * as fs from 'fs';
 import * as nock from 'nock';
+import { ATTR_HTTP_RESPONSE_STATUS_CODE } from '@opentelemetry/semantic-conventions';
 
 const region = 'us-east-1';
 
@@ -77,6 +78,11 @@ describe('instrumentation-aws-sdk-v3 (client-s3)', () => {
       expect(span.name).toEqual('S3.PutObject');
       expect(span.kind).toEqual(SpanKind.CLIENT);
       expect(span.attributes[ATTR_HTTP_STATUS_CODE]).toEqual(200);
+
+      // We also expect the stable `http.response.status_code` because
+      // `OTEL_SEMCONV_STABILITY_OPT_IN=http/dup` has been set before creating
+      // the instrumentation.
+      expect(span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]).toEqual(200);
     });
 
     it('callback interface', done => {
@@ -103,6 +109,7 @@ describe('instrumentation-aws-sdk-v3 (client-s3)', () => {
         expect(span.attributes[AttributeNames.CLOUD_REGION]).toEqual(region);
         expect(span.name).toEqual('S3.PutObject');
         expect(span.attributes[ATTR_HTTP_STATUS_CODE]).toEqual(200);
+        expect(span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]).toEqual(200);
         done();
       });
     });
@@ -132,6 +139,7 @@ describe('instrumentation-aws-sdk-v3 (client-s3)', () => {
       expect(span.attributes[AttributeNames.CLOUD_REGION]).toEqual(region);
       expect(span.name).toEqual('S3.PutObject');
       expect(span.attributes[ATTR_HTTP_STATUS_CODE]).toEqual(200);
+      expect(span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]).toEqual(200);
     });
 
     it('aws error', async () => {
@@ -166,6 +174,7 @@ describe('instrumentation-aws-sdk-v3 (client-s3)', () => {
           'invalid-bucket-name'
         );
         expect(span.attributes[ATTR_HTTP_STATUS_CODE]).toEqual(403);
+        expect(span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]).toEqual(403);
         expect(span.attributes[AttributeNames.CLOUD_REGION]).toEqual(region);
         expect(span.attributes[AttributeNames.AWS_REQUEST_ID]).toEqual(
           'MS95GTS7KXQ34X2S'

packages/instrumentation-aws-sdk/test/aws-sdk-v3-sqs.test.ts

@@ -30,7 +30,10 @@ process.env.AWS_SECRET_ACCESS_KEY = 'testing';
 import 'mocha';
 import { ReadableSpan } from '@opentelemetry/sdk-trace-base';
 import { context, trace } from '@opentelemetry/api';
-import { ATTR_URL_FULL } from '@opentelemetry/semantic-conventions';
+import {
+  ATTR_HTTP_RESPONSE_STATUS_CODE,
+  ATTR_URL_FULL,
+} from '@opentelemetry/semantic-conventions';
 import {
   ATTR_HTTP_STATUS_CODE,
   ATTR_MESSAGING_BATCH_MESSAGE_COUNT,
@@ -95,6 +98,7 @@ describe('instrumentation-aws-sdk-v3 (client-sqs)', () => {
       response.MessageId
     );
     expect(span.attributes[ATTR_HTTP_STATUS_CODE]).toEqual(200);
+    expect(span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]).toEqual(200);
   });
 
   it('sqs send message batch attributes', async () => {
@@ -145,6 +149,7 @@ describe('instrumentation-aws-sdk-v3 (client-sqs)', () => {
     );
     expect(span.attributes[ATTR_URL_FULL]).toEqual(params.QueueUrl);
     expect(span.attributes[ATTR_HTTP_STATUS_CODE]).toEqual(200);
+    expect(span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]).toEqual(200);
   });
 
   it('sqs receive add messaging attributes', done => {
@@ -178,6 +183,7 @@ describe('instrumentation-aws-sdk-v3 (client-sqs)', () => {
       expect(span.attributes[ATTR_RPC_SERVICE]).toEqual('SQS');
       expect(span.attributes[AttributeNames.CLOUD_REGION]).toEqual(region);
       expect(span.attributes[ATTR_HTTP_STATUS_CODE]).toEqual(200);
+      expect(span.attributes[ATTR_HTTP_RESPONSE_STATUS_CODE]).toEqual(200);
       expect(span.attributes[ATTR_MESSAGING_BATCH_MESSAGE_COUNT]).toEqual(2);
       expect(span.links.length).toBe(2);
 

packages/instrumentation-aws-sdk/test/load-instrumentation.ts

@@ -30,6 +30,11 @@ import {
 } from '@opentelemetry/sdk-metrics';
 import { AwsInstrumentation } from '../src';
 
+// This is a meagre testing of just a single value of
+// OTEL_SEMCONV_STABILITY_OPT_IN, because testing multiple configurations of
+// `AwsInstrumentation` in this all-in-one-process test setup is difficult.
+process.env.OTEL_SEMCONV_STABILITY_OPT_IN = 'http/dup';
+
 export const instrumentation = new AwsInstrumentation();
 export const metricExporter = new InMemoryMetricExporter(
   AggregationTemporality.DELTA