Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .oxlintrc.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,16 @@
"no-param-reassign": "off"
}
},
{
"files": ["**/integrations/tracing/redis/vendored/**/*.ts"],
"rules": {
"typescript/no-explicit-any": "off",
"typescript/no-unsafe-member-access": "off",
"typescript/no-this-alias": "off",
"max-lines": "off",
"no-bitwise": "off"
}
},
{
"files": [
"**/scenarios/**",
Expand Down
2 changes: 0 additions & 2 deletions packages/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@
"@opentelemetry/instrumentation-graphql": "0.62.0",
"@opentelemetry/instrumentation-hapi": "0.60.0",
"@opentelemetry/instrumentation-http": "0.214.0",
"@opentelemetry/instrumentation-ioredis": "0.62.0",
"@opentelemetry/instrumentation-kafkajs": "0.23.0",
"@opentelemetry/instrumentation-knex": "0.58.0",
"@opentelemetry/instrumentation-koa": "0.62.0",
Expand All @@ -86,7 +85,6 @@
"@opentelemetry/instrumentation-mysql": "0.60.0",
"@opentelemetry/instrumentation-mysql2": "0.60.0",
"@opentelemetry/instrumentation-pg": "0.66.0",
"@opentelemetry/instrumentation-redis": "0.62.0",
"@opentelemetry/instrumentation-tedious": "0.33.0",
"@opentelemetry/sdk-trace-base": "^2.6.1",
"@opentelemetry/semantic-conventions": "^1.40.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import type { Span } from '@opentelemetry/api';
import type { RedisResponseCustomAttributeFunction } from '@opentelemetry/instrumentation-ioredis';
import { IORedisInstrumentation } from '@opentelemetry/instrumentation-ioredis';
import { RedisInstrumentation } from '@opentelemetry/instrumentation-redis';
import type { IntegrationFn } from '@sentry/core';
import {
defineIntegration,
Expand All @@ -14,14 +11,18 @@ import {
truncate,
} from '@sentry/core';
import { generateInstrumentOnce } from '@sentry/node-core';
import type { IORedisCommandArgs } from '../../../utils/redisCache';
import {
calculateCacheItemSize,
GET_COMMANDS,
getCacheKeySafely,
getCacheOperation,
isInCommands,
shouldConsiderForCache,
} from '../../utils/redisCache';
} from '../../../utils/redisCache';
import type { IORedisInstrumentationConfig } from './vendored/types';
import { IORedisInstrumentation } from './vendored/ioredis-instrumentation';
import { RedisInstrumentation } from './vendored/redis-instrumentation';

interface RedisOptions {
/**
Expand All @@ -46,11 +47,11 @@ const INTEGRATION_NAME = 'Redis';
export let _redisOptions: RedisOptions = {};

/* Only exported for testing purposes */
export const cacheResponseHook: RedisResponseCustomAttributeFunction = (
export const cacheResponseHook: IORedisInstrumentationConfig['responseHook'] = (
span: Span,
redisCommand,
cmdArgs,
response,
redisCommand: string,
cmdArgs: IORedisCommandArgs,
response: unknown,
) => {
span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.redis');

Comment on lines 47 to 57
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The cacheResponseHook may silently fail to add network attributes to Redis v4/v5 cache spans because options.socket properties are not always available.
Severity: MEDIUM

Suggested Fix

To ensure network attributes are consistently added, the logic for Redis v4/v5 should be updated to find connection details from a more reliable source on the client object if options.socket.host and options.socket.port are not available. This will make the attribute collection more robust across different client configurations.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/node/src/integrations/tracing/redis/index.ts#L47-L57

Potential issue: The `cacheResponseHook` is now applied to `RedisInstrumentation` for
Redis v2-v5. For Redis v4/v5, the hook relies on `net.peer.name` and `net.peer.port`
attributes being set on the command span. These attributes are sourced from
`options?.socket?.host` and `options?.socket?.port`. However, these properties are not
guaranteed to be populated in a standard `node-redis` v4/v5 client configuration. If
they are missing, the attributes will not be set, and the hook will silently fail to add
`network.peer.address` and `network.peer.port` to the resulting cache spans, leading to
incomplete tracing data.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,259 @@
/*
* Copyright The OpenTelemetry Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* NOTICE from the Sentry authors:
* - Vendored from: https://github.com/open-telemetry/opentelemetry-js-contrib/tree/instrumentation-ioredis-v0.62.0/packages/instrumentation-ioredis
* - Upstream version: @opentelemetry/instrumentation-ioredis@0.62.0
* - Minor TypeScript adjustments for this repository's compiler settings
*/
/* eslint-disable -- vendored @opentelemetry/instrumentation-ioredis */

import { context, diag, SpanKind, SpanStatusCode, trace } from '@opentelemetry/api';
import type { Span } from '@opentelemetry/api';
import {
InstrumentationBase,
InstrumentationNodeModuleDefinition,
isWrapped,
safeExecuteInTheMiddle,
SemconvStability,
semconvStabilityFromStr,
} from '@opentelemetry/instrumentation';
import {
ATTR_DB_QUERY_TEXT,
ATTR_DB_SYSTEM_NAME,
ATTR_SERVER_ADDRESS,
ATTR_SERVER_PORT,
} from '@opentelemetry/semantic-conventions';

import { defaultDbStatementSerializer } from './redis-common';
import {
ATTR_DB_CONNECTION_STRING,
ATTR_DB_STATEMENT,
ATTR_DB_SYSTEM,
ATTR_NET_PEER_NAME,
ATTR_NET_PEER_PORT,
DB_SYSTEM_NAME_VALUE_REDIS,
DB_SYSTEM_VALUE_REDIS,
} from './semconv';
import type { IORedisInstrumentationConfig } from './types';

const PACKAGE_NAME = '@opentelemetry/instrumentation-ioredis';
const PACKAGE_VERSION = '0.62.0';

// ---- utils ----

function endSpan(span: Span, err: Error | null | undefined): void {
if (err) {
span.recordException(err);
span.setStatus({
code: SpanStatusCode.ERROR,
message: err.message,
});
}
span.end();
}

// ---- IORedisInstrumentation ----

const DEFAULT_CONFIG: IORedisInstrumentationConfig = {
requireParentSpan: true,
};

export class IORedisInstrumentation extends InstrumentationBase<IORedisInstrumentationConfig> {
_netSemconvStability!: SemconvStability;
_dbSemconvStability!: SemconvStability;

constructor(config: IORedisInstrumentationConfig = {}) {
super(PACKAGE_NAME, PACKAGE_VERSION, { ...DEFAULT_CONFIG, ...config });
this._setSemconvStabilityFromEnv();
}

_setSemconvStabilityFromEnv(): void {
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 = {}): void {
super.setConfig({ ...DEFAULT_CONFIG, ...config });
}

init() {
return [
new InstrumentationNodeModuleDefinition(
'ioredis',
['>=2.0.0 <6'],
(module: any, moduleVersion?: string) => {
const moduleExports =
module[Symbol.toStringTag] === 'Module'
? module.default // ESM
: module; // CommonJS
if (isWrapped(moduleExports.prototype.sendCommand)) {
this._unwrap(moduleExports.prototype, 'sendCommand');
}
this._wrap(moduleExports.prototype, 'sendCommand', this._patchSendCommand(moduleVersion));
if (isWrapped(moduleExports.prototype.connect)) {
this._unwrap(moduleExports.prototype, 'connect');
}
this._wrap(moduleExports.prototype, 'connect', this._patchConnection());
return module;
},
(module: any) => {
if (module === undefined) return;
const moduleExports =
module[Symbol.toStringTag] === 'Module'
? module.default // ESM
: module; // CommonJS
this._unwrap(moduleExports.prototype, 'sendCommand');
this._unwrap(moduleExports.prototype, 'connect');
},
),
];
}

private _patchSendCommand(moduleVersion?: string) {
return (original: Function) => {
return this._traceSendCommand(original, moduleVersion);
};
}

private _patchConnection() {
return (original: Function) => {
return this._traceConnection(original);
};
}

private _traceSendCommand(original: Function, moduleVersion?: string) {
const instrumentation = this;
return function (this: any, cmd: any) {
if (arguments.length < 1 || typeof cmd !== 'object') {
return original.apply(this, arguments);
}
const config = instrumentation.getConfig();
const dbStatementSerializer = config.dbStatementSerializer || defaultDbStatementSerializer;
const hasNoParentSpan = trace.getSpan(context.active()) === undefined;
if (config.requireParentSpan === true && hasNoParentSpan) {
return original.apply(this, arguments);
}
const attributes: Record<string, any> = {};
const { host, port } = this.options;
const dbQueryText = dbStatementSerializer(cmd.name, cmd.args);
if (instrumentation._dbSemconvStability & SemconvStability.OLD) {
attributes[ATTR_DB_SYSTEM] = DB_SYSTEM_VALUE_REDIS;
attributes[ATTR_DB_STATEMENT] = dbQueryText;
attributes[ATTR_DB_CONNECTION_STRING] = `redis://${host}:${port}`;
}
if (instrumentation._dbSemconvStability & SemconvStability.STABLE) {
attributes[ATTR_DB_SYSTEM_NAME] = DB_SYSTEM_NAME_VALUE_REDIS;
attributes[ATTR_DB_QUERY_TEXT] = dbQueryText;
}
if (instrumentation._netSemconvStability & SemconvStability.OLD) {
attributes[ATTR_NET_PEER_NAME] = host;
attributes[ATTR_NET_PEER_PORT] = port;
}
if (instrumentation._netSemconvStability & SemconvStability.STABLE) {
attributes[ATTR_SERVER_ADDRESS] = host;
attributes[ATTR_SERVER_PORT] = port;
}
const span = instrumentation.tracer.startSpan(cmd.name, {
kind: SpanKind.CLIENT,
attributes,
});
const { requestHook } = config;
if (requestHook) {
safeExecuteInTheMiddle(
() =>
requestHook(span, {
moduleVersion,
cmdName: cmd.name,
cmdArgs: cmd.args,
}),
(e: Error | undefined) => {
if (e) {
diag.error('ioredis instrumentation: request hook failed', e);
}
},
true,
);
}
try {
const result = original.apply(this, arguments);
const origResolve = cmd.resolve;
cmd.resolve = function (result: unknown) {
safeExecuteInTheMiddle(
() => config.responseHook?.(span, cmd.name, cmd.args, result),
(e: Error | undefined) => {
if (e) {
diag.error('ioredis instrumentation: response hook failed', e);
}
},
true,
);
endSpan(span, null);
origResolve(result);
};
const origReject = cmd.reject;
cmd.reject = function (err: Error) {
endSpan(span, err);
origReject(err);
};
return result;
} catch (error) {
endSpan(span, error as Error);
throw error;
}
};
}

private _traceConnection(original: Function) {
const instrumentation = this;
return function (this: any) {
const hasNoParentSpan = trace.getSpan(context.active()) === undefined;
if (instrumentation.getConfig().requireParentSpan === true && hasNoParentSpan) {
return original.apply(this, arguments);
}
const attributes: Record<string, any> = {};
const { host, port } = this.options;
if (instrumentation._dbSemconvStability & SemconvStability.OLD) {
attributes[ATTR_DB_SYSTEM] = DB_SYSTEM_VALUE_REDIS;
attributes[ATTR_DB_STATEMENT] = 'connect';
attributes[ATTR_DB_CONNECTION_STRING] = `redis://${host}:${port}`;
}
if (instrumentation._dbSemconvStability & SemconvStability.STABLE) {
attributes[ATTR_DB_SYSTEM_NAME] = DB_SYSTEM_NAME_VALUE_REDIS;
attributes[ATTR_DB_QUERY_TEXT] = 'connect';
}
if (instrumentation._netSemconvStability & SemconvStability.OLD) {
attributes[ATTR_NET_PEER_NAME] = host;
attributes[ATTR_NET_PEER_PORT] = port;
}
if (instrumentation._netSemconvStability & SemconvStability.STABLE) {
attributes[ATTR_SERVER_ADDRESS] = host;
attributes[ATTR_SERVER_PORT] = port;
}
const span = instrumentation.tracer.startSpan('connect', {
kind: SpanKind.CLIENT,
attributes,
});
try {
const client = original.apply(this, arguments);
endSpan(span, null);
return client;
} catch (error) {
endSpan(span, error as Error);
throw error;
}
};
}
}
Loading
Loading