diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts index 4bf503e6c4..07628f2055 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts @@ -354,7 +354,7 @@ export class RedisInstrumentation extends InstrumentationBase { return function patchedConnect(this: any): Promise { const options = this.options; - const attributes = getClientAttributes(options); + const attributes = getClientAttributes(this._diag, options); const span = plugin.tracer.startSpan( `${RedisInstrumentation.COMPONENT}-connect`, @@ -405,7 +405,7 @@ export class RedisInstrumentation extends InstrumentationBase { const dbStatementSerializer = this._config?.dbStatementSerializer || defaultDbStatementSerializer; - const attributes = getClientAttributes(clientOptions); + const attributes = getClientAttributes(this._diag, clientOptions); try { const dbStatement = dbStatementSerializer(commandName, commandArgs); diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts b/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts index f102606474..336bf7f2b2 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts @@ -13,16 +13,45 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { DiagLogger } from '@opentelemetry/api'; import { DbSystemValues, SemanticAttributes, } from '@opentelemetry/semantic-conventions'; -export function getClientAttributes(options: any) { +export function getClientAttributes(diag: DiagLogger, options: any) { return { [SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS, [SemanticAttributes.NET_PEER_NAME]: options?.socket?.host, [SemanticAttributes.NET_PEER_PORT]: options?.socket?.port, - [SemanticAttributes.DB_CONNECTION_STRING]: options?.url, + [SemanticAttributes.DB_CONNECTION_STRING]: + removeCredentialsFromDBConnectionStringAttribute(diag, options?.url), }; } + +/** + * removeCredentialsFromDBConnectionStringAttribute removes basic auth from url and user_pwd from query string + * + * Examples: + * redis://user:pass@localhost:6379/mydb => redis://localhost:6379/mydb + * redis://localhost:6379?db=mydb&user_pwd=pass => redis://localhost:6379?db=mydb + */ +function removeCredentialsFromDBConnectionStringAttribute( + diag: DiagLogger, + url?: unknown +): string | undefined { + if (typeof url !== 'string') { + return; + } + + try { + const u = new URL(url); + u.searchParams.delete('user_pwd'); + u.username = ''; + u.password = ''; + return u.href; + } catch (err) { + diag.error('failed to sanitize redis connection url', err); + } + return; +} diff --git a/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts index 615572cf5a..ac5abf5bce 100644 --- a/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts @@ -220,8 +220,11 @@ describe('redis@^4.0.0', () => { }); it('sets error status on connection failure', async () => { + const redisURL = `redis://${redisTestConfig.host}:${ + redisTestConfig.port + 1 + }`; const newClient = createClient({ - url: `redis://${redisTestConfig.host}:${redisTestConfig.port + 1}`, + url: redisURL, }); await assert.rejects(newClient.connect()); @@ -230,6 +233,64 @@ describe('redis@^4.0.0', () => { assert.strictEqual(span.name, 'redis-connect'); assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual( + span.attributes[SemanticAttributes.DB_CONNECTION_STRING], + redisURL + ); + }); + + it('omits basic auth from DB_CONNECTION_STRING span attribute', async () => { + const redisURL = `redis://myuser:mypassword@${redisTestConfig.host}:${ + redisTestConfig.port + 1 + }`; + const expectAttributeConnString = `redis://${redisTestConfig.host}:${ + redisTestConfig.port + 1 + }`; + const newClient = createClient({ + url: redisURL, + }); + + await assert.rejects(newClient.connect()); + + const [span] = getTestSpans(); + + assert.strictEqual(span.name, 'redis-connect'); + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual( + span.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + span.attributes[SemanticAttributes.DB_CONNECTION_STRING], + expectAttributeConnString + ); + }); + + it('omits user_pwd query parameter from DB_CONNECTION_STRING span attribute', async () => { + const redisURL = `redis://${redisTestConfig.host}:${ + redisTestConfig.port + 1 + }?db=mydb&user_pwd=mypassword`; + const expectAttributeConnString = `redis://${redisTestConfig.host}:${ + redisTestConfig.port + 1 + }?db=mydb`; + const newClient = createClient({ + url: redisURL, + }); + + await assert.rejects(newClient.connect()); + + const [span] = getTestSpans(); + + assert.strictEqual(span.name, 'redis-connect'); + assert.strictEqual(span.status.code, SpanStatusCode.ERROR); + assert.strictEqual( + span.attributes[SemanticAttributes.NET_PEER_NAME], + redisTestConfig.host + ); + assert.strictEqual( + span.attributes[SemanticAttributes.DB_CONNECTION_STRING], + expectAttributeConnString + ); }); });