Skip to content

Commit

Permalink
fix(instrumentation-redis): Take host and port used for connection
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Zirak committed Apr 2, 2024
1 parent a9b1063 commit c9676fe
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
* limitations under the License.
*/
export interface RedisPluginClientTypes {
options?: {
host: string;
port: string;
connection_options?: {
port?: string;
host?: string;
};

address?: string;
Expand Down
6 changes: 3 additions & 3 deletions plugins/node/opentelemetry-instrumentation-redis/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down

0 comments on commit c9676fe

Please sign in to comment.