feat(instrumentation-pg): update to stable semantic conventions with db-migration support (#2881)

Co-authored-by: Trent Mick <[email protected]>

Author: maryliag

package-lock.json

@@ -51,6 +51,48 @@ PostgreSQL instrumentation has few options available to choose from. You can set
 | `requireParentSpan` | `boolean` | If true, requires a parent span to create new spans (default false) |
 | `addSqlCommenterCommentToQueries` | `boolean` | If true, adds [sqlcommenter](https://github.com/open-telemetry/opentelemetry-sqlcommenter) specification compliant comment to queries with tracing context (default false). _NOTE: A comment will not be added to queries that already contain `--` or `/* ... */` in them, even if these are not actually part of comments_ |
 
+## Semantic Conventions
+
+Prior to version `0.55.0`, this instrumentation created spans and metrics targeting an experimental semantic convention Version 1.27.0.
+
+Database semantic conventions (semconv) were stabilized in v1.34.0, and a [migration process](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/non-normative/db-migration.md) was defined.
+`@opentelemetry/instrumentation-pg` versions 0.55.0 and later include support for migrating to stable Database semantic conventions, as described below.
+The intent is to provide an approximate 6 month time window for users of this instrumentation to migrate to the new Database semconv, after which a new minor version will use the new semconv by default and drop support for the old semconv.
+
+To select which semconv version(s) is emitted from this instrumentation, use the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable.
+
+- `database`: emit the new (stable) v1.34.0+ semantics
+- `database/dup`: emit **both** the old v1.27.0 and the new (stable) v1.34.0+ semantics
+- By default, if `OTEL_SEMCONV_STABILITY_OPT_IN` includes neither of the above tokens, the old v1.27.0 semconv is used.
+
+### Attributes collected
+
+| v1.27.0 semconv         | v1.34.0 semconv                                 | Short Description                                                                          |
+| ----------------------- | ----------------------------------------------- | ------------------------------------------------------------------------------------------ |
+| `db.connection_string`  | Removed                                         | String used to connect to the database                                                     |
+| `db.user`               | Removed                                         | User used to connect to the database                                                       |
+| `db.name`               | Removed, integrated into the new `db.namespace` | The name of the database.                                                                  |
+| (not included)          | `db.namespace`                                  | The name of the database, fully qualified within the server address and port.              |
+| `db.statement`          | `db.query.text`                                 | The database query being executed.                                                         |
+| `db.system`             | `db.system.name`                                | The database management system (DBMS) product as identified by the client instrumentation. |
+| `net.peer.port`         | `server.port`                                   | Remote port number.                                                                        |
+| `net.peer.name`         | `server.address`                                | Remote hostname or similar.                                                                |
+
+Metrics Exported:
+
+- [`db.client.operation.duration`](https://github.com/open-telemetry/semantic-conventions/blob/main/docs/database/database-metrics.md#metric-dbclientoperationduration)
+
+### Upgrading Semantic Conventions
+
+When upgrading to the new semantic conventions, it is recommended to do so in the following order:
+
+1. Upgrade `@opentelemetry/opentelemetry-instrumentation-pg` to the latest version
+2. Set `OTEL_SEMCONV_STABILITY_OPT_IN=database/dup` to emit both old and new semantic conventions
+3. Modify alerts, dashboards, metrics, and other processes to expect the new semantic conventions
+4. Set `OTEL_SEMCONV_STABILITY_OPT_IN=database` to emit only the new semantic conventions
+
+This will cause both the old and new semantic conventions to be emitted during the transition period.
+
 ## Useful links
 
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>

packages/instrumentation-pg/README.md

@@ -75,7 +75,7 @@
   "dependencies": {
     "@opentelemetry/core": "^2.0.0",
     "@opentelemetry/instrumentation": "^0.203.0",
-    "@opentelemetry/semantic-conventions": "^1.27.0",
+    "@opentelemetry/semantic-conventions": "^1.34.0",
     "@opentelemetry/sql-common": "^0.41.0",
     "@types/pg": "8.15.4",
     "@types/pg-pool": "2.0.6"

packages/instrumentation-pg/package.json

@@ -19,6 +19,8 @@ import {
   InstrumentationNodeModuleDefinition,
   safeExecuteInTheMiddle,
   InstrumentationNodeModuleFile,
+  SemconvStability,
+  semconvStabilityFromStr,
 } from '@opentelemetry/instrumentation';
 import {
   context,
@@ -54,18 +56,19 @@ import {
   hrTimeToMilliseconds,
 } from '@opentelemetry/core';
 import {
-  DBSYSTEMVALUES_POSTGRESQL,
-  SEMATTRS_DB_SYSTEM,
   ATTR_ERROR_TYPE,
   ATTR_SERVER_PORT,
   ATTR_SERVER_ADDRESS,
+  ATTR_DB_NAMESPACE,
+  ATTR_DB_OPERATION_NAME,
+  ATTR_DB_SYSTEM_NAME,
+  METRIC_DB_CLIENT_OPERATION_DURATION,
 } from '@opentelemetry/semantic-conventions';
 import {
   METRIC_DB_CLIENT_CONNECTION_COUNT,
   METRIC_DB_CLIENT_CONNECTION_PENDING_REQUESTS,
-  METRIC_DB_CLIENT_OPERATION_DURATION,
-  ATTR_DB_NAMESPACE,
-  ATTR_DB_OPERATION_NAME,
+  ATTR_DB_SYSTEM,
+  DB_SYSTEM_VALUE_POSTGRESQL,
 } from './semconv';
 
 function extractModuleExports(module: any) {
@@ -88,9 +91,14 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
     idle: 0,
     pending: 0,
   };
+  private _semconvStability: SemconvStability;
 
   constructor(config: PgInstrumentationConfig = {}) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
+    this._semconvStability = semconvStabilityFromStr(
+      'database',
+      process.env.OTEL_SEMCONV_STABILITY_OPT_IN
+    );
   }
 
   override _updateMetricInstruments() {
@@ -247,7 +255,10 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
 
         const span = plugin.tracer.startSpan(SpanNames.CONNECT, {
           kind: SpanKind.CLIENT,
-          attributes: utils.getSemanticAttributesFromConnection(this),
+          attributes: utils.getSemanticAttributesFromConnection(
+            this,
+            plugin._semconvStability
+          ),
         });
 
         if (callback) {
@@ -272,14 +283,19 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
 
   private recordOperationDuration(attributes: Attributes, startTime: HrTime) {
     const metricsAttributes: Attributes = {};
-    const keysToCopy = [
-      SEMATTRS_DB_SYSTEM,
+    const keysToCopy: string[] = [
       ATTR_DB_NAMESPACE,
       ATTR_ERROR_TYPE,
       ATTR_SERVER_PORT,
       ATTR_SERVER_ADDRESS,
       ATTR_DB_OPERATION_NAME,
     ];
+    if (this._semconvStability & SemconvStability.OLD) {
+      keysToCopy.push(ATTR_DB_SYSTEM);
+    }
+    if (this._semconvStability & SemconvStability.STABLE) {
+      keysToCopy.push(ATTR_DB_SYSTEM_NAME);
+    }
 
     keysToCopy.forEach(key => {
       if (key in attributes) {
@@ -327,7 +343,7 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
           : undefined;
 
         const attributes: Attributes = {
-          [SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_POSTGRESQL,
+          [ATTR_DB_SYSTEM]: DB_SYSTEM_VALUE_POSTGRESQL,
           [ATTR_DB_NAMESPACE]: this.database,
           [ATTR_SERVER_PORT]: this.connectionParameters.port,
           [ATTR_SERVER_ADDRESS]: this.connectionParameters.host,
@@ -348,6 +364,7 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
           this,
           plugin.tracer,
           instrumentationConfig,
+          plugin._semconvStability,
           queryConfig
         );
 
@@ -548,7 +565,10 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
         // setup span
         const span = plugin.tracer.startSpan(SpanNames.POOL_CONNECT, {
           kind: SpanKind.CLIENT,
-          attributes: utils.getSemanticAttributesFromPool(this.options),
+          attributes: utils.getSemanticAttributesFromPoolConnection(
+            this.options,
+            plugin._semconvStability
+          ),
         });
 
         plugin._setPoolConnectEventListeners(this);

packages/instrumentation-pg/src/instrumentation.ts

@@ -32,6 +32,7 @@ export type PostgresCallback = (err: Error, res: object) => unknown;
 export interface PgParsedConnectionParams {
   database?: string;
   host?: string;
+  namespace?: string;
   port?: number;
   user?: string;
 }
@@ -47,17 +48,18 @@ export type PgPoolCallback = (
 ) => void;
 
 export interface PgPoolOptionsParams {
+  allowExitOnIdle: boolean;
+  connectionString?: string; // connection string if provided directly
   database: string;
   host: string;
-  port: number;
-  user: string;
   idleTimeoutMillis: number; // the minimum amount of time that an object may sit idle in the pool before it is eligible for eviction due to idle time
-  maxClient: number; // maximum size of the pool
-  connectionString?: string; // connection string if provided directly
   max: number;
-  maxUses: number;
-  allowExitOnIdle: boolean;
+  maxClient: number; // maximum size of the pool
   maxLifetimeSeconds: number;
+  maxUses: number;
+  namespace: string;
+  port: number;
+  user: string;
 }
 
 export const EVENT_LISTENERS_SET = Symbol(

packages/instrumentation-pg/src/internal-types.ts

@@ -14,6 +14,12 @@
  * limitations under the License.
  */
 
+/*
+ * This file contains a copy of unstable semantic convention definitions
+ * used by this package.
+ * @see https://github.com/open-telemetry/opentelemetry-js/tree/main/semantic-conventions#unstable-semconv
+ */
+
 /**
  * The name of the connection pool; unique within the instrumented application. In case the connection pool implementation doesn't provide a name, instrumentation **SHOULD** use a combination of parameters that would make the name unique, for example, combining attributes `server.address`, `server.port`, and `db.namespace`, formatted as `server.address:server.port/db.namespace`. Instrumentations that generate connection pool name following different patterns **SHOULD** document it.
  *
@@ -22,7 +28,7 @@
  * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
  */
 export const ATTR_DB_CLIENT_CONNECTION_POOL_NAME =
-  'db.client.connection.pool.name';
+  'db.client.connection.pool.name' as const;
 
 /**
  * The state of a connection in the pool
@@ -31,70 +37,114 @@ export const ATTR_DB_CLIENT_CONNECTION_POOL_NAME =
  *
  * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
  */
-export const ATTR_DB_CLIENT_CONNECTION_STATE = 'db.client.connection.state';
+export const ATTR_DB_CLIENT_CONNECTION_STATE =
+  'db.client.connection.state' as const;
+
+/**
+ * Deprecated, use `server.address`, `server.port` attributes instead.
+ *
+ * @example "Server=(localdb)\\v11.0;Integrated Security=true;"
+ *
+ * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
+ *
+ * @deprecated Replaced by `server.address` and `server.port`.
+ */
+export const ATTR_DB_CONNECTION_STRING = 'db.connection_string' as const;
 
 /**
- * The name of the database, fully qualified within the server address and port.
+ * Deprecated, use `db.namespace` instead.
  *
  * @example customers
- * @example test.users
+ * @example main
+ *
+ * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
  *
- * @note If a database system has multiple namespace components, they **SHOULD** be concatenated (potentially using database system specific conventions) from most general to most specific namespace component, and more specific namespaces **SHOULD NOT** be captured without the more general namespaces, to ensure that "startswith" queries for the more general namespaces will be valid.
- * Semantic conventions for individual database systems **SHOULD** document what `db.namespace` means in the context of that system.
- * It is **RECOMMENDED** to capture the value as provided by the application without attempting to do any case normalization.
- * This attribute has stability level RELEASE CANDIDATE.
+ * @deprecated Replaced by `db.namespace`.
+ */
+export const ATTR_DB_NAME = 'db.name' as const;
+
+/**
+ * The database statement being executed.
+ *
+ * @example SELECT * FROM wuser_table
+ * @example SET mykey "WuValue"
  *
  * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
+ *
+ * @deprecated Replaced by `db.query.text`.
  */
-export const ATTR_DB_NAMESPACE = 'db.namespace';
+export const ATTR_DB_STATEMENT = 'db.statement' as const;
 
 /**
- * The name of the operation or command being executed.
+ * Deprecated, use `db.system.name` instead.
+ *
+ * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
  *
- * @example findAndModify
- * @example HMSET
- * @example SELECT
+ * @deprecated Replaced by `db.system.name`.
+ */
+export const ATTR_DB_SYSTEM = 'db.system' as const;
+
+/**
+ * Deprecated, no replacement at this time.
  *
- * @note It is **RECOMMENDED** to capture the value as provided by the application without attempting to do any case normalization.
- * If the operation name is parsed from the query text, it **SHOULD** be the first operation name found in the query.
- * For batch operations, if the individual operations are known to have the same operation name then that operation name **SHOULD** be used prepended by `BATCH `, otherwise `db.operation.name` **SHOULD** be `BATCH` or some other database system specific term if more applicable.
- * This attribute has stability level RELEASE CANDIDATE.
+ * @example readonly_user
+ * @example reporting_user
  *
  * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
+ *
+ * @deprecated Removed, no replacement at this time.
  */
-export const ATTR_DB_OPERATION_NAME = 'db.operation.name';
+export const ATTR_DB_USER = 'db.user' as const;
 
 /**
- * Enum value "used" for attribute {@link ATTR_DB_CLIENT_CONNECTION_STATE}.
+ * Deprecated, use `server.address` on client spans and `client.address` on server spans.
+ *
+ * @example example.com
+ *
+ * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
+ *
+ * @deprecated Replaced by `server.address` on client spans and `client.address` on server spans.
+ */
+export const ATTR_NET_PEER_NAME = 'net.peer.name' as const;
+
+/**
+ * Deprecated, use `server.port` on client spans and `client.port` on server spans.
+ *
+ * @example 8080
+ *
+ * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
+ *
+ * @deprecated Replaced by `server.port` on client spans and `client.port` on server spans.
  */
-export const DB_CLIENT_CONNECTION_STATE_VALUE_USED = 'used';
+export const ATTR_NET_PEER_PORT = 'net.peer.port' as const;
 
 /**
  * Enum value "idle" for attribute {@link ATTR_DB_CLIENT_CONNECTION_STATE}.
  */
-export const DB_CLIENT_CONNECTION_STATE_VALUE_IDLE = 'idle';
+export const DB_CLIENT_CONNECTION_STATE_VALUE_IDLE = 'idle' as const;
 
 /**
- * The number of connections that are currently in state described by the `state` attribute
- *
- * @experimental This metric is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
+ * Enum value "used" for attribute {@link ATTR_DB_CLIENT_CONNECTION_STATE}.
  */
-export const METRIC_DB_CLIENT_CONNECTION_COUNT = 'db.client.connection.count';
+export const DB_CLIENT_CONNECTION_STATE_VALUE_USED = 'used' as const;
 
 /**
- * The number of current pending requests for an open connection
+ * Enum value "postgresql" for attribute {@link ATTR_DB_SYSTEM}.
+ */
+export const DB_SYSTEM_VALUE_POSTGRESQL = 'postgresql' as const;
+
+/**
+ * The number of connections that are currently in state described by the `state` attribute
  *
  * @experimental This metric is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
  */
-export const METRIC_DB_CLIENT_CONNECTION_PENDING_REQUESTS =
-  'db.client.connection.pending_requests';
+export const METRIC_DB_CLIENT_CONNECTION_COUNT =
+  'db.client.connection.count' as const;
 
 /**
- * Duration of database client operations.
- *
- * @note Batch operations **SHOULD** be recorded as a single operation.
+ * The number of current pending requests for an open connection
  *
  * @experimental This metric is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
  */
-export const METRIC_DB_CLIENT_OPERATION_DURATION =
-  'db.client.operation.duration';
+export const METRIC_DB_CLIENT_CONNECTION_PENDING_REQUESTS =
+  'db.client.connection.pending_requests' as const;