From f7c6f6f06e2969f8b9b7e6e98aafd7a460589dea Mon Sep 17 00:00:00 2001 From: Zirak Date: Fri, 29 Mar 2024 16:35:35 +0300 Subject: [PATCH 1/2] fix(instrumentation-redis): Take host and port used for connection The Redis client allows specifying connection options in several ways, with sensible defaults. The following all translate into `127.0.0.1:6379`: ```js createClient('redis://127.0.0.1:6379'); createClient({ host: '127.0.0.1', port: NaN }); createClient({}) createClient() ``` Redis somewhat normalises these separate options into its `options` member, and stores the properties it uses to connect to the server in `connection_options`. Examples of the difference between the two in the examples preceding (slightly redacted for ease of reading): ```js createClient('redis://127.0.0.1:6379'); // options = { port: '6379', host: '127.0.0.1' } // connection_options = { port: 6379, host: '127.0.0.1', family: 4 } createClient({ host: '127.0.0.1', port: NaN }); // options = { host: '127.0.0.1', port: NaN } // connection_options = { port: 6379, host: '127.0.0.1', family: 4 } createClient() // options = { host: undefined } // connection_options = { port: 6379, host: '127.0.0.1', family: 4 } ``` The instrumentation before this commit looks at the `options` property, which contains caller-supplied values before they're fully normalised and smoothed over by Redis. This means that for these weird cases, the instrumentation would populate `NET_PEER_NAME` and `NET_PEER_PORT` with erroneous values. This commit has the instrumentation the values the Redis client uses to connect to the server, mirroring actual use. --- .../src/internal-types.ts | 6 +++--- .../node/opentelemetry-instrumentation-redis/src/utils.ts | 6 +++--- .../opentelemetry-instrumentation-redis/test/redis.test.ts | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/internal-types.ts b/plugins/node/opentelemetry-instrumentation-redis/src/internal-types.ts index 99e2efe829..545a1927e4 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/internal-types.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/internal-types.ts @@ -14,9 +14,9 @@ * limitations under the License. */ export interface RedisPluginClientTypes { - options?: { - host: string; - port: string; + connection_options?: { + port?: string; + host?: string; }; address?: string; diff --git a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts index 7575e51376..493a0022bb 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/src/utils.ts @@ -110,10 +110,10 @@ export const getTracedInternalSendCommand = ( ); // Set attributes for not explicitly typed RedisPluginClientTypes - if (this.options) { + if (this.connection_options) { span.setAttributes({ - [SemanticAttributes.NET_PEER_NAME]: this.options.host, - [SemanticAttributes.NET_PEER_PORT]: this.options.port, + [SemanticAttributes.NET_PEER_NAME]: this.connection_options.host, + [SemanticAttributes.NET_PEER_PORT]: this.connection_options.port, }); } if (this.address) { diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts index 01d449a57e..1c6cefe068 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts @@ -47,7 +47,7 @@ const memoryExporter = new InMemorySpanExporter(); const CONFIG = { host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', - port: process.env.OPENTELEMETRY_REDIS_PORT || '63790', + port: process.env.OPENTELEMETRY_REDIS_PORT || 63790, }; const URL = `redis://${CONFIG.host}:${CONFIG.port}`; From 498d9682923818659ac6590b9286867b98327d7b Mon Sep 17 00:00:00 2001 From: Zirak Date: Wed, 17 Apr 2024 09:24:28 +0300 Subject: [PATCH 2/2] test(instrumentation-redis): Explicitly expect for a numeric port --- .../node/opentelemetry-instrumentation-redis/test/redis.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts index 1c6cefe068..736f8d76f3 100644 --- a/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis/test/redis.test.ts @@ -47,7 +47,7 @@ const memoryExporter = new InMemorySpanExporter(); const CONFIG = { host: process.env.OPENTELEMETRY_REDIS_HOST || 'localhost', - port: process.env.OPENTELEMETRY_REDIS_PORT || 63790, + port: Number(process.env.OPENTELEMETRY_REDIS_PORT || 63790), }; const URL = `redis://${CONFIG.host}:${CONFIG.port}`;