fix(redis): apply small review fixes

Author: adbista

packages/instrumentation-redis/src/types.ts

@@ -60,6 +60,11 @@ export interface RedisInstrumentationConfig extends InstrumentationConfig {
   /** Require parent to create redis span, default when unset is false */
   requireParentSpan?: boolean;
 
-  /** Controls which semantic convention attributes are emitted on spans. */
+  /**
+   * Controls which semantic-convention attributes are emitted on spans.
+   * Default: 'OLD'.
+   * When this option is set, it takes precedence over any value provided via
+   * the OTEL_SEMCONV_STABILITY_OPT_IN environment variable.
+   */
   semconvStability?: SemconvStability;
 }

packages/instrumentation-redis/src/v2-v3/instrumentation.ts

@@ -31,7 +31,7 @@ import { RedisInstrumentationConfig } from '../types';
 /** @knipignore */
 import { PACKAGE_NAME, PACKAGE_VERSION } from '../version';
 import type { RedisCommand, RedisPluginClientTypes } from './internal-types';
-import { SpanKind, context, trace } from '@opentelemetry/api';
+import { Attributes, SpanKind, context, trace } from '@opentelemetry/api';
 import {
   DBSYSTEMVALUES_REDIS,
   SEMATTRS_DB_CONNECTION_STRING,
@@ -53,10 +53,12 @@ export class RedisInstrumentationV2_V3 extends InstrumentationBase<RedisInstrume
 
   constructor(config: RedisInstrumentationConfig = {}) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
-    this._semconvStability = semconvStabilityFromStr(
-      'database',
-      process.env.OTEL_SEMCONV_STABILITY_OPT_IN
-    );
+    this._semconvStability = config.semconvStability
+      ? config.semconvStability
+      : semconvStabilityFromStr(
+          'database',
+          process.env.OTEL_SEMCONV_STABILITY_OPT_IN
+        );
   }
 
   override setConfig(config: RedisInstrumentationConfig = {}) {
@@ -150,7 +152,7 @@ export class RedisInstrumentationV2_V3 extends InstrumentationBase<RedisInstrume
         const dbStatementSerializer =
           config?.dbStatementSerializer || defaultDbStatementSerializer;
 
-        const attributes: { [key: string]: any } = {};
+        const attributes: Attributes = {};
 
         if (instrumentation._semconvStability & SemconvStability.OLD) {
           Object.assign(attributes, {
@@ -180,7 +182,7 @@ export class RedisInstrumentationV2_V3 extends InstrumentationBase<RedisInstrume
 
         // Set attributes for not explicitly typed RedisPluginClientTypes
         if (this.connection_options) {
-          const connectionAttributes: { [key: string]: any } = {};
+          const connectionAttributes: Attributes = {};
 
           if (instrumentation._semconvStability & SemconvStability.OLD) {
             Object.assign(connectionAttributes, {

packages/instrumentation-redis/src/v4-v5/instrumentation.ts

@@ -37,7 +37,6 @@ import { PACKAGE_NAME, PACKAGE_VERSION } from '../version';
 import {
   ATTR_DB_OPERATION_NAME,
   ATTR_DB_QUERY_TEXT,
-  ATTR_DB_OPERATION_BATCH_SIZE,
   SEMATTRS_DB_STATEMENT,
 } from '@opentelemetry/semantic-conventions';
 import type { MultiErrorReply } from './internal-types';
@@ -61,10 +60,12 @@ export class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrume
 
   constructor(config: RedisInstrumentationConfig = {}) {
     super(PACKAGE_NAME, PACKAGE_VERSION, config);
-    this._semconvStability = semconvStabilityFromStr(
-      'database',
-      process.env.OTEL_SEMCONV_STABILITY_OPT_IN
-    );
+    this._semconvStability = config.semconvStability
+      ? config.semconvStability
+      : semconvStabilityFromStr(
+          'database',
+          process.env.OTEL_SEMCONV_STABILITY_OPT_IN
+        );
   }
 
   override setConfig(config: RedisInstrumentationConfig = {}) {
@@ -409,11 +410,10 @@ export class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrume
       clientOptions,
       this._semconvStability
     );
-
+    if (this._semconvStability & SemconvStability.STABLE) {
+      attributes[ATTR_DB_OPERATION_NAME] = commandName;
+    }
     try {
-      if (this._semconvStability & SemconvStability.STABLE) {
-        attributes[ATTR_DB_OPERATION_NAME] = commandName;
-      }
       const dbStatement = dbStatementSerializer(commandName, commandArgs);
       if (dbStatement != null) {
         if (this._semconvStability & SemconvStability.OLD) {
@@ -487,12 +487,6 @@ export class RedisInstrumentationV4_V5 extends InstrumentationBase<RedisInstrume
 
     for (let i = 0; i < openSpans.length; i++) {
       const { span, commandName, commandArgs } = openSpans[i];
-      if (
-        openSpans.length >= 2 &&
-        this._semconvStability & SemconvStability.STABLE
-      ) {
-        span.setAttribute(ATTR_DB_OPERATION_BATCH_SIZE, openSpans.length);
-      }
       const currCommandRes = replies[i];
       const [res, err] =
         currCommandRes instanceof Error

packages/instrumentation-redis/test/v4-v5/redis.test.ts

@@ -52,7 +52,6 @@ import {
   ATTR_SERVER_ADDRESS,
   ATTR_SERVER_PORT,
   ATTR_EXCEPTION_MESSAGE,
-  ATTR_DB_OPERATION_BATCH_SIZE,
 } from '@opentelemetry/semantic-conventions';
 import { RedisResponseCustomAttributeFunction } from '../../src/types';
 import { hrTimeToMilliseconds, suppressTracing } from '@opentelemetry/core';
@@ -663,28 +662,6 @@ describe('redis v4-v5', () => {
     });
   });
 
-  describe('extra coverage batch size and URL sanitization', () => {
-    const stableCfg = { semconvStability: SemconvStability.STABLE };
-
-    afterEach(() => {
-      instrumentation.setConfig({});
-    });
-
-    it('should set ATTR_DB_OPERATION_BATCH_SIZE when multi has 3 commands', async () => {
-      instrumentation.setConfig(stableCfg);
-      const multi = client.multi();
-      multi.set('covKey1', 'v1');
-      multi.get('covKey1');
-      multi.set('covKey2', 'v2');
-      await multi.exec();
-
-      const spanWithBatch = getTestSpans().find(
-        s => s.attributes[ATTR_DB_OPERATION_BATCH_SIZE] === 3
-      );
-      assert.ok(spanWithBatch, 'No span had batch size 3');
-    });
-  });
-
   describe('config', () => {
     describe('dbStatementSerializer', () => {
       it('custom dbStatementSerializer', async () => {