feat(instrumentations-tedious): support net.* and database semconv migration

packages/instrumentation-tedious/README.md

@@ -42,19 +42,29 @@ registerInstrumentations({
 
 ## 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 instrumentation implements Semantic Conventions (semconv) v1.7.0. Since then, networking (in semconv v1.23.1) and database (in semconv v1.33.0) semantic conventions were stabilized. As of `@opentelemetry/instrumentation-ioredis@0.57.0` support has been added for migrating to the stable semantic conventions using the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable as follows:
+
+1. Upgrade to the latest version of this instrumentation package.
+2. Set `OTEL_SEMCONV_STABILITY_OPT_IN=http/dup,database/dup` to emit both old and stable semantic conventions. (The `http` token is used to control the `net.*` attributes, the `database` token to control to `db.*` attributes.)
+3. Modify alerts, dashboards, metrics, and other processes in your Observability system to use the stable semantic conventions.
+4. Set `OTEL_SEMCONV_STABILITY_OPT_IN=http,database` to emit only the stable semantic conventions.
+
+By default, if `OTEL_SEMCONV_STABILITY_OPT_IN` includes neither of the above tokens, the old v1.7.0 semconv is used.
+The intent is to provide an approximate 6 month time window for users of this instrumentation to migrate to the new database and networking semconv, after which a new minor version will use the new semconv by default and drop support for the old semconv.
+See [the HTTP migration guide](https://opentelemetry.io/docs/specs/semconv/non-normative/http-migration/) and the [database migration guide](https://opentelemetry.io/docs/specs/semconv/non-normative/db-migration/) for details.
 
 Attributes collected:
 
-| Attribute               | Short Description                                                              |
-| ----------------------- | ------------------------------------------------------------------------------ |
-| `db.name`               | This attribute is used to report the name of the database being accessed.      |
-| `db.sql.table`          | The name of the primary table that the operation is acting upon.               |
-| `db.statement`          | The database statement being executed.                                         |
-| `db.system`             | An identifier for the database management system (DBMS) product being used.    |
-| `db.user`               | Username for accessing the database.                                           |
-| `net.peer.name`         | Remote hostname or similar.                                                    |
-| `net.peer.port`         | Remote port number.                                                            |
+| Old semconv     | Stable semconv       | Description                        |
+| --------------- | -------------------- | ---------------------------------- |
+| `db.system`     | `db.system.name`     | 'mssql' (old), 'microsoft.sql_server' (stable) |
+| `db.statement`  | `db.query.text`      | The database query being executed. |
+| `db.user`       | Removed              | Username for accessing the database. |
+| `db.name`       | Removed              | Integrated into new `db.namespace`. |
+| (not included)  | `db.namespace`       | The database associated with the connection, qualified by the instance name. |
+| `db.sql.table`  | `db.collection.name` | The name of a collection (table, container) within the database. |
+| `net.peer.name` | `server.address`     | Remote hostname or similar.        |
+| `net.peer.port` | `server.port`        | Remote port number.                |
 
 ### Trace Context Propagation
 

packages/instrumentation-tedious/package.json

@@ -60,6 +60,7 @@
   },
   "dependencies": {
     "@opentelemetry/instrumentation": "^0.208.0",
+    "@opentelemetry/semantic-conventions": "^1.33.0",
     "@types/tedious": "^4.0.14"
   },
   "homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/instrumentation-tedious#readme"

packages/instrumentation-tedious/src/instrumentation.ts

@@ -20,7 +20,18 @@ import {
   InstrumentationBase,
   InstrumentationNodeModuleDefinition,
   isWrapped,
+  SemconvStability,
+  semconvStabilityFromStr,
 } from '@opentelemetry/instrumentation';
+import {
+  ATTR_DB_COLLECTION_NAME,
+  ATTR_DB_NAMESPACE,
+  ATTR_DB_QUERY_TEXT,
+  ATTR_DB_SYSTEM_NAME,
+  ATTR_SERVER_ADDRESS,
+  ATTR_SERVER_PORT,
+  DB_SYSTEM_NAME_VALUE_MICROSOFT_SQL_SERVER,
+} from '@opentelemetry/semantic-conventions';
 import {
   DB_SYSTEM_VALUE_MSSQL,
   ATTR_DB_NAME,
@@ -75,9 +86,24 @@ function setDatabase(this: ApproxConnection, databaseName: string) {
 
 export class TediousInstrumentation extends InstrumentationBase<TediousInstrumentationConfig> {
   static readonly COMPONENT = 'tedious';
+  private _netSemconvStability!: SemconvStability;
+  private _dbSemconvStability!: SemconvStability;
 
   constructor(config: TediousInstrumentationConfig = {}) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
+    this._setSemconvStabilityFromEnv();
+  }
+
+  // Used for testing.
+  private _setSemconvStabilityFromEnv() {
+    this._netSemconvStability = semconvStabilityFromStr(
+      'http',
+      process.env.OTEL_SEMCONV_STABILITY_OPT_IN
+    );
+    this._dbSemconvStability = semconvStabilityFromStr(
+      'database',
+      process.env.OTEL_SEMCONV_STABILITY_OPT_IN
+    );
   }
 
   protected init() {
@@ -209,22 +235,44 @@ export class TediousInstrumentation extends InstrumentationBase<TediousInstrumen
           return request.sqlTextOrProcedure;
         })(request);
 
+        const attributes: api.Attributes = {};
+        if (thisPlugin._dbSemconvStability & SemconvStability.OLD) {
+          attributes[ATTR_DB_SYSTEM] = DB_SYSTEM_VALUE_MSSQL;
+          attributes[ATTR_DB_NAME] = databaseName;
+          // >=4 uses `authentication` object; older versions just userName and password pair
+          attributes[ATTR_DB_USER] =
+            this.config?.userName ??
+            this.config?.authentication?.options?.userName;
+          attributes[ATTR_DB_STATEMENT] = sql;
+          attributes[ATTR_DB_SQL_TABLE] = request.table;
+        }
+        if (thisPlugin._dbSemconvStability & SemconvStability.STABLE) {
+          // The OTel spec for "db.namespace" discusses handling for connection
+          // to MSSQL "named instances". This isn't currently supported.
+          //    https://opentelemetry.io/docs/specs/semconv/database/sql-server/#:~:text=%5B1%5D%20db%2Enamespace
+          attributes[ATTR_DB_NAMESPACE] = databaseName;
+          attributes[ATTR_DB_SYSTEM_NAME] = DB_SYSTEM_NAME_VALUE_MICROSOFT_SQL_SERVER;
+          attributes[ATTR_DB_QUERY_TEXT] = sql;
+          attributes[ATTR_DB_COLLECTION_NAME] = request.table;
+          // See https://opentelemetry.io/docs/specs/semconv/database/sql-server/#spans
+          // TODO: can `db.response.status_code` be added?
+          // TODO: is `operation` correct for `db.operation.name`
+          // TODO: can `db.query.summary` reliably be calculated?
+          // TODO: `db.stored_procedure.name`
+        }
+        if (thisPlugin._netSemconvStability & SemconvStability.OLD) {
+          attributes[ATTR_NET_PEER_NAME] = this.config?.server;
+          attributes[ATTR_NET_PEER_PORT] = this.config?.options?.port;
+        }
+        if (thisPlugin._netSemconvStability & SemconvStability.STABLE) {
+          attributes[ATTR_SERVER_ADDRESS] = this.config?.server;
+          attributes[ATTR_SERVER_PORT] = this.config?.options?.port;
+        }
         const span = thisPlugin.tracer.startSpan(
           getSpanName(operation, databaseName, sql, request.table),
           {
             kind: api.SpanKind.CLIENT,
-            attributes: {
-              [ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_MSSQL,
-              [ATTR_DB_NAME]: databaseName,
-              [ATTR_NET_PEER_PORT]: this.config?.options?.port,
-              [ATTR_NET_PEER_NAME]: this.config?.server,
-              // >=4 uses `authentication` object, older versions just userName and password pair
-              [ATTR_DB_USER]:
-                this.config?.userName ??
-                this.config?.authentication?.options?.userName,
-              [ATTR_DB_STATEMENT]: sql,
-              [ATTR_DB_SQL_TABLE]: request.table,
-            },
+            attributes,
           }
         );
 
@@ -242,6 +290,7 @@ export class TediousInstrumentation extends InstrumentationBase<TediousInstrumen
               code: api.SpanStatusCode.ERROR,
               message: err.message,
             });
+            // TODO: set `error.type` attribute?
           }
           span.end();
         });
@@ -279,7 +328,7 @@ export class TediousInstrumentation extends InstrumentationBase<TediousInstrumen
         if (!shouldInject) return runUserRequest();
 
         const traceparent = thisPlugin._buildTraceparent(span);
-        
+
         void thisPlugin
           ._injectContextInfo(this, tediousModule, traceparent)
           .finally(runUserRequest);

packages/instrumentation-tedious/test/instrumentation.test.ts

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { context, trace, SpanStatusCode, SpanKind } from '@opentelemetry/api';
+import { context, trace, SpanStatusCode, SpanKind, type Attributes } from '@opentelemetry/api';
 import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
 import {
   DB_SYSTEM_VALUE_MSSQL,
@@ -26,7 +26,7 @@ import {
   ATTR_NET_PEER_NAME,
   ATTR_NET_PEER_PORT,
 } from '../src/semconv';
-import * as util from 'util';
+import { SemconvStability } from '@opentelemetry/instrumentation';
 import * as testUtils from '@opentelemetry/contrib-test-utils';
 import {
   BasicTracerProvider,
@@ -39,6 +39,13 @@ import { TediousInstrumentation } from '../src';
 import makeApi from './api';
 import type { Connection } from 'tedious';
 import * as semver from 'semver';
+// XXX has prettier config stopped working in linting?
+import { ATTR_DB_COLLECTION_NAME, ATTR_DB_NAMESPACE, ATTR_DB_QUERY_TEXT, ATTR_DB_SYSTEM_NAME, ATTR_SERVER_ADDRESS, ATTR_SERVER_PORT, DB_SYSTEM_NAME_VALUE_MICROSOFT_SQL_SERVER } from '@opentelemetry/semantic-conventions';
+
+// By default tests run with both old and stable semconv. Some test cases
+// specifically test the various values of OTEL_SEMCONV_STABILITY_OPT_IN.
+process.env.OTEL_SEMCONV_STABILITY_OPT_IN = 'http/dup,database/dup';
+const DEFAULT_NET_SEMCONV_STABILITY = SemconvStability.DUPLICATE;
 
 const port = Number(process.env.MSSQL_PORT) || 1433;
 const database = process.env.MSSQL_DATABASE || 'master';
@@ -307,6 +314,65 @@ describe('tedious', () => {
     });
   });
 
+  describe('various values of OTEL_SEMCONV_STABILITY_OPT_IN', () => {
+    const _origOptInEnv = process.env.OTEL_SEMCONV_STABILITY_OPT_IN;
+    after(() => {
+      process.env.OTEL_SEMCONV_STABILITY_OPT_IN = _origOptInEnv;
+      (instrumentation as any)._setSemconvStabilityFromEnv();
+    });
+
+    it('OTEL_SEMCONV_STABILITY_OPT_IN=(empty)', async () => {
+      process.env.OTEL_SEMCONV_STABILITY_OPT_IN = '';
+      (instrumentation as any)._setSemconvStabilityFromEnv();
+      memoryExporter.reset();
+
+      const queryString = "SELECT 42, 'hello world'";
+      const PARENT_NAME = 'parentSpan';
+      const parentSpan = provider.getTracer('default').startSpan(PARENT_NAME);
+      assert.deepStrictEqual(
+        await context.with(trace.setSpan(context.active(), parentSpan), () =>
+          tedious.query(connection, queryString)
+        ),
+        [42, 'hello world']
+      );
+      parentSpan.end();
+      const spans = memoryExporter.getFinishedSpans();
+      assert.strictEqual(spans.length, 2, 'Received incorrect number of spans');
+      assertSpan(spans[0], {
+        name: 'execSql master',
+        sql: queryString,
+        parentSpan,
+      }, SemconvStability.OLD);
+      assert.strictEqual(spans[1].name, PARENT_NAME);
+    });
+
+    it('OTEL_SEMCONV_STABILITY_OPT_IN=http,database', async () => {
+      process.env.OTEL_SEMCONV_STABILITY_OPT_IN = 'http,database';
+      (instrumentation as any)._setSemconvStabilityFromEnv();
+      memoryExporter.reset();
+
+      const queryString = "SELECT 42, 'hello world'";
+      const PARENT_NAME = 'parentSpan';
+      const parentSpan = provider.getTracer('default').startSpan(PARENT_NAME);
+      assert.deepStrictEqual(
+        await context.with(trace.setSpan(context.active(), parentSpan), () =>
+          tedious.query(connection, queryString)
+        ),
+        [42, 'hello world']
+      );
+      parentSpan.end();
+      const spans = memoryExporter.getFinishedSpans();
+      assert.strictEqual(spans.length, 2, 'Received incorrect number of spans');
+      assertSpan(spans[0], {
+        name: 'execSql master',
+        sql: queryString,
+        parentSpan,
+      }, SemconvStability.STABLE);
+      assert.strictEqual(spans[1].name, PARENT_NAME);
+    });
+  });
+
+
   describe('trace context propagation via CONTEXT_INFO', () => {
     function traceparentFromSpan(span: ReadableSpan) {
       const sc = span.spanContext();
@@ -370,13 +436,6 @@ describe('tedious', () => {
   });
 });
 
-const assertMatch = (actual: string | undefined, expected: RegExp) => {
-  assert(
-    actual && expected.test(actual),
-    `Expected ${util.inspect(actual)} to match ${expected}`
-  );
-};
-
 const assertRejects = (
   asyncFn: () => Promise<unknown>,
   expectedMessageRegexp: RegExp | undefined
@@ -392,52 +451,75 @@ const assertRejects = (
         throw error;
       }
       if (expectedMessageRegexp) {
-        assertMatch(err?.message || err, expectedMessageRegexp);
+        assert.match(err?.message || err, expectedMessageRegexp);
       }
     });
 };
 
-function assertSpan(span: ReadableSpan, expected: any) {
-  assert(span);
+function assertSpan(
+  span: ReadableSpan, expected: any, semconvStability: SemconvStability = DEFAULT_NET_SEMCONV_STABILITY)
+{
+  assert.ok(span);
   assert.strictEqual(span.name, expected.name);
   assert.strictEqual(span.kind, SpanKind.CLIENT);
-  assert.strictEqual(span.attributes[ATTR_DB_SYSTEM], DB_SYSTEM_VALUE_MSSQL);
-  assert.strictEqual(
-    span.attributes[ATTR_DB_NAME],
-    expected.database ?? database
-  );
-  assert.strictEqual(span.attributes[ATTR_NET_PEER_PORT], port);
-  assert.strictEqual(span.attributes[ATTR_NET_PEER_NAME], host);
-  assert.strictEqual(span.attributes[ATTR_DB_USER], user);
-  assert.strictEqual(
-    span.attributes['tedious.procedure_count'],
-    expected.procCount ?? 1,
-    'Invalid procedure_count'
-  );
-  assert.strictEqual(
-    span.attributes['tedious.statement_count'],
-    expected.statementCount ?? 1,
-    'Invalid statement_count'
-  );
+
+  // Attributes
+  const actualAttrs = {...span.attributes};
+  const expectedAttrs: Attributes = {
+    'tedious.procedure_count': expected.procCount ?? 1,
+    'tedious.statement_count': expected.statementCount ?? 1,
+  };
+  if (semconvStability & SemconvStability.OLD) {
+    expectedAttrs[ATTR_DB_SYSTEM] = DB_SYSTEM_VALUE_MSSQL;
+    expectedAttrs[ATTR_DB_NAME] = expected.database ?? database;
+    expectedAttrs[ATTR_DB_USER] = user;
+    expectedAttrs[ATTR_NET_PEER_NAME] = host;
+    expectedAttrs[ATTR_NET_PEER_PORT] = port;
+    if (expected.table) {
+      expectedAttrs[ATTR_DB_SQL_TABLE] = expected.table;
+    }
+    // "db.statement"
+    if (expected.sql) {
+      if (expected.sql instanceof RegExp) {
+        assert.match(span.attributes[ATTR_DB_STATEMENT] as string, expected.sql);
+      } else {
+        assert.strictEqual(span.attributes[ATTR_DB_STATEMENT], expected.sql, ATTR_DB_STATEMENT);
+      }
+    } else {
+      assert.strictEqual(actualAttrs[ATTR_DB_STATEMENT], undefined);
+    }
+    delete actualAttrs[ATTR_DB_STATEMENT];
+  }
+  if (semconvStability & SemconvStability.STABLE) {
+    expectedAttrs[ATTR_DB_SYSTEM_NAME] = DB_SYSTEM_NAME_VALUE_MICROSOFT_SQL_SERVER;
+    expectedAttrs[ATTR_DB_NAMESPACE] = expected.database ?? database;
+    expectedAttrs[ATTR_SERVER_ADDRESS] = host;
+    expectedAttrs[ATTR_SERVER_PORT] = port;
+    if (expected.table) {
+      expectedAttrs[ATTR_DB_COLLECTION_NAME] = expected.table;
+    }
+    // "db.statement"
+    if (expected.sql) {
+      if (expected.sql instanceof RegExp) {
+        assert.match(span.attributes[ATTR_DB_QUERY_TEXT] as string, expected.sql);
+      } else {
+        assert.strictEqual(span.attributes[ATTR_DB_QUERY_TEXT], expected.sql, ATTR_DB_QUERY_TEXT);
+      }
+    } else {
+      assert.strictEqual(actualAttrs[ATTR_DB_QUERY_TEXT], undefined);
+    }
+    delete actualAttrs[ATTR_DB_QUERY_TEXT];
+  }
+  assert.deepEqual(actualAttrs, expectedAttrs);
+
+
   if (expected.parentSpan) {
     assert.strictEqual(
       span.parentSpanContext?.spanId,
       expected.parentSpan.spanContext().spanId
     );
   }
-  assert.strictEqual(span.attributes[ATTR_DB_SQL_TABLE], expected.table);
-  if (expected.sql) {
-    if (expected.sql instanceof RegExp) {
-      assertMatch(
-        span.attributes[ATTR_DB_STATEMENT] as string | undefined,
-        expected.sql
-      );
-    } else {
-      assert.strictEqual(span.attributes[ATTR_DB_STATEMENT], expected.sql);
-    }
-  } else {
-    assert.strictEqual(span.attributes[ATTR_DB_STATEMENT], undefined);
-  }
+
   if (expected.error) {
     assert(
       expected.error.test(span.status.message),