From 001304272626611d86eb6d227f53af03c2b000d4 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 2 Dec 2025 11:41:32 -0800 Subject: [PATCH 1/2] feat(instrumentations-ioredis): support `net.*` and database semconv migration This adds support for using `OTEL_SEMCONV_STABILITY_OPT_IN` for controlled migration to stable `net.*` and `db.*` semconv. The `net.*` attributes are controlled by the `http[/dup]` token in `OTEL_SEMCONV_STABILITY_OPT_IN` (as [discussed here](https://github.com/open-telemetry/opentelemetry-js/issues/5663#issuecomment-3314043915)) and `db.*` with the `database[/dup]` token. Refs: https://github.com/open-telemetry/opentelemetry-js/issues/5663 Refs: https://github.com/open-telemetry/opentelemetry-js-contrib/issues/2953 --- package-lock.json | 4 +- packages/instrumentation-ioredis/README.md | 26 ++- packages/instrumentation-ioredis/package.json | 4 +- .../src/instrumentation.ts | 89 ++++++-- .../instrumentation-ioredis/src/semconv.ts | 9 + .../test/ioredis.test.ts | 202 ++++++++++++++---- 6 files changed, 259 insertions(+), 75 deletions(-) diff --git a/package-lock.json b/package-lock.json index 0338948434..504c0e4609 100644 --- a/package-lock.json +++ b/package-lock.json @@ -38139,7 +38139,8 @@ "license": "Apache-2.0", "dependencies": { "@opentelemetry/instrumentation": "^0.208.0", - "@opentelemetry/redis-common": "^0.38.2" + "@opentelemetry/redis-common": "^0.38.2", + "@opentelemetry/semantic-conventions": "^1.33.0" }, "devDependencies": { "@opentelemetry/api": "^1.3.0", @@ -38147,7 +38148,6 @@ "@opentelemetry/contrib-test-utils": "^0.55.0", "@opentelemetry/sdk-trace-base": "^2.0.0", "@opentelemetry/sdk-trace-node": "^2.0.0", - "@opentelemetry/semantic-conventions": "^1.27.0", "@types/ioredis4": "npm:@types/ioredis@4.28.10", "ioredis": "5.8.2" }, diff --git a/packages/instrumentation-ioredis/README.md b/packages/instrumentation-ioredis/README.md index ff1265ddc2..74d6d20df8 100644 --- a/packages/instrumentation-ioredis/README.md +++ b/packages/instrumentation-ioredis/README.md @@ -100,17 +100,27 @@ requestHook: function ( ## 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.connection_string` | The connection string used to connect to the database. | -| `db.statement` | The database statement being executed. | -| `db.system` | An identifier for the database management system (DBMS) product being used. | -| `net.peer.name` | Remote hostname or similar. | -| `net.peer.port` | Remote port number. | +| Old semconv | Stable semconv | Description | +| ---------------------- | ---------------- | ----------- | +| `db.connection_string` | Removed | The connection string used to connect to the database. | +| `db.system` | `db.system.name` | 'redis' | +| `db.statement` | `db.query.text` | The database query being executed. | +| `net.peer.port` | `server.port` | Remote port number. | +| `net.peer.name` | `server.address` | Remote hostname or similar. | + ## Useful links diff --git a/packages/instrumentation-ioredis/package.json b/packages/instrumentation-ioredis/package.json index 0e8219dbeb..b987485d87 100644 --- a/packages/instrumentation-ioredis/package.json +++ b/packages/instrumentation-ioredis/package.json @@ -56,13 +56,13 @@ "@opentelemetry/contrib-test-utils": "^0.55.0", "@opentelemetry/sdk-trace-base": "^2.0.0", "@opentelemetry/sdk-trace-node": "^2.0.0", - "@opentelemetry/semantic-conventions": "^1.27.0", "@types/ioredis4": "npm:@types/ioredis@4.28.10", "ioredis": "5.8.2" }, "dependencies": { "@opentelemetry/instrumentation": "^0.208.0", - "@opentelemetry/redis-common": "^0.38.2" + "@opentelemetry/redis-common": "^0.38.2", + "@opentelemetry/semantic-conventions": "^1.33.0" }, "homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/instrumentation-ioredis#readme" } diff --git a/packages/instrumentation-ioredis/src/instrumentation.ts b/packages/instrumentation-ioredis/src/instrumentation.ts index 07e14fe02c..175f418530 100644 --- a/packages/instrumentation-ioredis/src/instrumentation.ts +++ b/packages/instrumentation-ioredis/src/instrumentation.ts @@ -14,16 +14,25 @@ * limitations under the License. */ -import { diag, trace, context, SpanKind } from '@opentelemetry/api'; +import { diag, trace, context, SpanKind, type Attributes } from '@opentelemetry/api'; import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped, + SemconvStability, + semconvStabilityFromStr, } from '@opentelemetry/instrumentation'; import { IORedisInstrumentationConfig } from './types'; import { IORedisCommand, RedisInterface } from './internal-types'; +import { + ATTR_DB_QUERY_TEXT, + ATTR_DB_SYSTEM_NAME, + ATTR_SERVER_ADDRESS, + ATTR_SERVER_PORT, +} from '@opentelemetry/semantic-conventions'; import { DB_SYSTEM_VALUE_REDIS, + DB_SYSTEM_NAME_VALUE_REDIS, ATTR_DB_CONNECTION_STRING, ATTR_DB_STATEMENT, ATTR_DB_SYSTEM, @@ -41,8 +50,24 @@ const DEFAULT_CONFIG: IORedisInstrumentationConfig = { }; export class IORedisInstrumentation extends InstrumentationBase { + private _netSemconvStability!: SemconvStability; + private _dbSemconvStability!: SemconvStability; + constructor(config: IORedisInstrumentationConfig = {}) { super(PACKAGE_NAME, PACKAGE_VERSION, { ...DEFAULT_CONFIG, ...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 + ); } override setConfig(config: IORedisInstrumentationConfig = {}) { @@ -120,12 +145,29 @@ export class IORedisInstrumentation extends InstrumentationBase { const span = provider.getTracer('ioredis-test').startSpan('test span'); let client: ioredisTypes.Redis; const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: 'connect', + [ATTR_DB_QUERY_TEXT]: 'connect', }; const readyHandler = () => { const endedSpans = memoryExporter.getFinishedSpans(); - - assert.strictEqual(trace.getSpan(context.active()), span); - assert.strictEqual(endedSpans.length, 2); - assert.strictEqual(endedSpans[0].name, 'connect'); - assert.strictEqual(endedSpans[1].name, 'info'); - testUtils.assertPropagation(endedSpans[0], span); - - testUtils.assertSpan( - endedSpans[0], - SpanKind.CLIENT, - attributes, - [], - unsetStatus - ); - span.end(); - assert.strictEqual(endedSpans.length, 3); - assert.strictEqual(endedSpans[2].name, 'test span'); - - client.quit(() => { - assert.strictEqual(endedSpans.length, 4); - assert.strictEqual(endedSpans[3].name, 'quit'); - done(); - }); + try { + assert.strictEqual(trace.getSpan(context.active()), span); + assert.strictEqual(endedSpans.length, 2); + assert.strictEqual(endedSpans[0].name, 'connect'); + assert.strictEqual(endedSpans[1].name, 'info'); + testUtils.assertPropagation(endedSpans[0], span); + + testUtils.assertSpan( + endedSpans[0], + SpanKind.CLIENT, + attributes, + [], + unsetStatus + ); + span.end(); + assert.strictEqual(endedSpans.length, 3); + assert.strictEqual(endedSpans[2].name, 'test span'); + } finally { + client.quit(() => { + assert.strictEqual(endedSpans.length, 4); + assert.strictEqual(endedSpans[3].name, 'quit'); + done(); + }); + } }; const errorHandler = (err: Error) => { assert.ifError(err); @@ -245,8 +262,10 @@ describe('ioredis', () => { IOREDIS_CALLBACK_OPERATIONS.forEach(command => { it(`should create a child span for cb style ${command.description}`, done => { const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: `${command.name} ${command.expectedDbStatement}`, + [ATTR_DB_QUERY_TEXT]: `${command.name} ${command.expectedDbStatement}`, }; const span = provider .getTracer('ioredis-test') @@ -275,8 +294,10 @@ describe('ioredis', () => { it('should create a child span for hset promise', async () => { const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: `hset ${hashKeyName} random [1 other arguments]`, + [ATTR_DB_QUERY_TEXT]: `hset ${hashKeyName} random [1 other arguments]`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -335,8 +356,10 @@ describe('ioredis', () => { it('should create a child span for streamify scanning', done => { const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000', + [ATTR_DB_QUERY_TEXT]: 'scan 0 MATCH test-* COUNT 1000', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { @@ -411,8 +434,10 @@ describe('ioredis', () => { ); const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: 'subscribe news music', + [ATTR_DB_QUERY_TEXT]: 'subscribe news music', }; testUtils.assertSpan( endedSpans[4], @@ -430,8 +455,10 @@ describe('ioredis', () => { it('should create a child span for multi/transaction', done => { const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: 'multi', + [ATTR_DB_QUERY_TEXT]: 'multi', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -466,8 +493,10 @@ describe('ioredis', () => { it('should create a child span for pipeline', done => { const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: 'set foo [1 other arguments]', + [ATTR_DB_QUERY_TEXT]: 'set foo [1 other arguments]', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -500,8 +529,10 @@ describe('ioredis', () => { it('should create a child span for get promise', async () => { const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: `get ${testKeyName}`, + [ATTR_DB_QUERY_TEXT]: `get ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -529,8 +560,10 @@ describe('ioredis', () => { it('should create a child span for del', async () => { const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: `del ${testKeyName}`, + [ATTR_DB_QUERY_TEXT]: `del ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -563,8 +596,10 @@ describe('ioredis', () => { instrumentation.setConfig(config); const attributes = { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`, + [ATTR_DB_QUERY_TEXT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -670,8 +705,10 @@ describe('ioredis', () => { endedSpans[0], SpanKind.CLIENT, { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`, + [ATTR_DB_QUERY_TEXT]: `set ${testKeyName} [1 other arguments]`, }, [], unsetStatus @@ -696,8 +733,10 @@ describe('ioredis', () => { endedSpans[0], SpanKind.CLIENT, { - ...DEFAULT_ATTRIBUTES, + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, [ATTR_DB_STATEMENT]: 'connect', + [ATTR_DB_QUERY_TEXT]: 'connect', }, [], unsetStatus @@ -744,12 +783,12 @@ describe('ioredis', () => { IOREDIS_CALLBACK_OPERATIONS.forEach(command => { it(`should tag the span with a custom db.statement for cb style ${command.description}`, done => { + const dbQueryText = dbStatementSerializer(command.name, command.args); const attributes = { - ...DEFAULT_ATTRIBUTES, - [ATTR_DB_STATEMENT]: dbStatementSerializer( - command.name, - command.args - ), + ...DEFAULT_OLD_ATTRIBUTES, + ...DEFAULT_STABLE_ATTRIBUTES, + [ATTR_DB_STATEMENT]: dbQueryText, + [ATTR_DB_QUERY_TEXT]: dbQueryText, }; const span = provider .getTracer('ioredis-test') @@ -972,6 +1011,9 @@ describe('ioredis', () => { before(() => { instrumentation.setConfig(config); }); + after(() => { + instrumentation.setConfig(); + }); IOREDIS_CALLBACK_OPERATIONS.forEach(operation => { it(`should properly execute the db statement serializer for operation ${operation.description}`, done => { @@ -1000,6 +1042,84 @@ describe('ioredis', () => { }); }); + describe('various values of OTEL_SEMCONV_STABILITY_OPT_IN', () => { + // use a random part in key names because redis instance is used for parallel running tests + const randomId = ((Math.random() * 2 ** 32) >>> 0).toString(16); + const testKeyName = `test-semconv-stability-${randomId}`; + + 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 () => { + // Arrange + process.env.OTEL_SEMCONV_STABILITY_OPT_IN = ''; + (instrumentation as any)._setSemconvStabilityFromEnv(); + memoryExporter.reset(); + + const client = new ioredis(REDIS_URL); + try { + // Act + const tracer = provider.getTracer('ioredis-test'); + await tracer.startActiveSpan('parent', async (parentSpan) => { + await client.set(testKeyName, 'aValue'); + parentSpan.end(); + }); + + // Assert + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 2); + testUtils.assertSpan( + spans[0], + SpanKind.CLIENT, + { + ...DEFAULT_OLD_ATTRIBUTES, + [ATTR_DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`, + }, + [], + unsetStatus + ); + } finally { + await client.quit(); + } + }); + + it('OTEL_SEMCONV_STABILITY_OPT_IN=http,database', async () => { + // Arrange + process.env.OTEL_SEMCONV_STABILITY_OPT_IN = 'http,database'; + (instrumentation as any)._setSemconvStabilityFromEnv(); + memoryExporter.reset(); + + const client = new ioredis(REDIS_URL); + try { + // Act + const tracer = provider.getTracer('ioredis-test'); + await tracer.startActiveSpan('parent', async (parentSpan) => { + await client.set(testKeyName, 'aValue'); + parentSpan.end(); + }); + + // Assert + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 2); + testUtils.assertSpan( + spans[0], + SpanKind.CLIENT, + { + ...DEFAULT_STABLE_ATTRIBUTES, + [ATTR_DB_QUERY_TEXT]: `set ${testKeyName} [1 other arguments]`, + }, + [], + unsetStatus + ); + } finally { + await client.quit(); + } + }); + }); + it('should work with ESM usage', async () => { await testUtils.runTestFixture({ cwd: __dirname, From b242deab16d7ac94adb1ec29adb4addc5f99c4ea Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 2 Dec 2025 12:15:23 -0800 Subject: [PATCH 2/2] pay obeisance to the markdownlint overlord --- packages/instrumentation-ioredis/README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/instrumentation-ioredis/README.md b/packages/instrumentation-ioredis/README.md index 74d6d20df8..3bae7aca98 100644 --- a/packages/instrumentation-ioredis/README.md +++ b/packages/instrumentation-ioredis/README.md @@ -113,13 +113,13 @@ See [the HTTP migration guide](https://opentelemetry.io/docs/specs/semconv/non-n Attributes collected: -| Old semconv | Stable semconv | Description | -| ---------------------- | ---------------- | ----------- | -| `db.connection_string` | Removed | The connection string used to connect to the database. | -| `db.system` | `db.system.name` | 'redis' | +| Old semconv | Stable semconv | Description | +| ---------------------- | ---------------- | ---------------------------------- | +| `db.connection_string` | Removed | | +| `db.system` | `db.system.name` | 'redis' | | `db.statement` | `db.query.text` | The database query being executed. | -| `net.peer.port` | `server.port` | Remote port number. | -| `net.peer.name` | `server.address` | Remote hostname or similar. | +| `net.peer.port` | `server.port` | Remote port number. | +| `net.peer.name` | `server.address` | Remote hostname or similar. | ## Useful links