Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore(instrumentation-ioredis): use exported strings for attributes #2191

Merged
merged 3 commits into from
May 14, 2024
Merged

chore(instrumentation-ioredis): use exported strings for attributes #2191

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e944b23
chore(instrumentation-ioredis): use exported strings for attributes
maryliag May 10, 2024
e063bae
update readme
maryliag May 11, 2024
a59a3a5
Merge branch 'main' into sem-ioredis
trentm May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions plugins/node/opentelemetry-instrumentation-ioredis/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,20 @@ requestHook: function (

```

## Semantic Conventions

This package uses `@opentelemetry/semantic-conventions` version `1.22+`, which implements Semantic Convention [Version 1.7.0](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.7.0/semantic_conventions/README.md)

Attributes collected:

| Attribute | Short Description |
|------------------------|-----------------------------------------------------------------------------|
| `db.connection_string` | The connection string used to connect to the database. |
| `db.statement` | The database statement being executed. |
| `db.system` | An identifier for the database management system (DBMS) product being used. |
| `net.peer.name` | Remote hostname or similar. |
| `net.peer.port` | Remote port number. |

## Useful links

- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
Expand Down
2 changes: 1 addition & 1 deletion plugins/node/opentelemetry-instrumentation-ioredis/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
"dependencies": {
"@opentelemetry/instrumentation": "^0.51.0",
"@opentelemetry/redis-common": "^0.36.2",
"@opentelemetry/semantic-conventions": "^1.0.0"
"@opentelemetry/semantic-conventions": "^1.23.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This should be ^1.22.0 right?

Suggested change
"@opentelemetry/semantic-conventions": "^1.23.0"
"@opentelemetry/semantic-conventions": "^1.22.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just though if we are making an update, might as well update to the latest, so I did this on purpose. The message on readme says about 1.22+ to accommodate when this would happen.

},
"homepage": "https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-ioredis#readme"
}
31 changes: 16 additions & 15 deletions plugins/node/opentelemetry-instrumentation-ioredis/src/instrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ import {
import { IORedisInstrumentationConfig } from './types';
import { IORedisCommand, RedisInterface } from './internal-types';
import {
DbSystemValues,
SemanticAttributes,
DBSYSTEMVALUES_REDIS,
SEMATTRS_DB_CONNECTION_STRING,
SEMATTRS_DB_STATEMENT,
SEMATTRS_DB_SYSTEM,
SEMATTRS_NET_PEER_NAME,
SEMATTRS_NET_PEER_PORT,
} from '@opentelemetry/semantic-conventions';
import { safeExecuteInTheMiddle } from '@opentelemetry/instrumentation';
import { endSpan } from './utils';
Expand Down Expand Up @@ -121,11 +125,8 @@ export class IORedisInstrumentation extends InstrumentationBase {
const span = instrumentation.tracer.startSpan(cmd.name, {
kind: SpanKind.CLIENT,
attributes: {
[SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS,
[SemanticAttributes.DB_STATEMENT]: dbStatementSerializer(
cmd.name,
cmd.args
),
[SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_REDIS,
[SEMATTRS_DB_STATEMENT]: dbStatementSerializer(cmd.name, cmd.args),
},
});

Expand All @@ -149,9 +150,9 @@ export class IORedisInstrumentation extends InstrumentationBase {
const { host, port } = this.options;

span.setAttributes({
[SemanticAttributes.NET_PEER_NAME]: host,
[SemanticAttributes.NET_PEER_PORT]: port,
[SemanticAttributes.DB_CONNECTION_STRING]: `redis://${host}:${port}`,
[SEMATTRS_NET_PEER_NAME]: host,
[SEMATTRS_NET_PEER_PORT]: port,
[SEMATTRS_DB_CONNECTION_STRING]: `redis://${host}:${port}`,
});

try {
Expand Down Expand Up @@ -201,16 +202,16 @@ export class IORedisInstrumentation extends InstrumentationBase {
const span = instrumentation.tracer.startSpan('connect', {
kind: SpanKind.CLIENT,
attributes: {
[SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS,
[SemanticAttributes.DB_STATEMENT]: 'connect',
[SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_REDIS,
[SEMATTRS_DB_STATEMENT]: 'connect',
},
});
const { host, port } = this.options;

span.setAttributes({
[SemanticAttributes.NET_PEER_NAME]: host,
[SemanticAttributes.NET_PEER_PORT]: port,
[SemanticAttributes.DB_CONNECTION_STRING]: `redis://${host}:${port}`,
[SEMATTRS_NET_PEER_NAME]: host,
[SEMATTRS_NET_PEER_PORT]: port,
[SEMATTRS_DB_CONNECTION_STRING]: `redis://${host}:${port}`,
});
try {
const client = original.apply(this, arguments);
Expand Down
67 changes: 35 additions & 32 deletions plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,15 @@ import {
IORedisRequestHookInformation,
} from '../src/types';
import {
DbSystemValues,
SemanticAttributes,
DBSYSTEMVALUES_REDIS,
SEMATTRS_DB_CONNECTION_STRING,
SEMATTRS_DB_STATEMENT,
SEMATTRS_DB_SYSTEM,
SEMATTRS_EXCEPTION_MESSAGE,
SEMATTRS_EXCEPTION_STACKTRACE,
SEMATTRS_EXCEPTION_TYPE,
SEMATTRS_NET_PEER_NAME,
SEMATTRS_NET_PEER_PORT,
} from '@opentelemetry/semantic-conventions';

const memoryExporter = new InMemorySpanExporter();
Expand All @@ -54,10 +61,10 @@ const CONFIG = {
const REDIS_URL = `redis://${CONFIG.host}:${CONFIG.port}`;

const DEFAULT_ATTRIBUTES = {
[SemanticAttributes.DB_SYSTEM]: DbSystemValues.REDIS,
[SemanticAttributes.NET_PEER_NAME]: CONFIG.host,
[SemanticAttributes.NET_PEER_PORT]: CONFIG.port,
[SemanticAttributes.DB_CONNECTION_STRING]: REDIS_URL,
[SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_REDIS,
[SEMATTRS_NET_PEER_NAME]: CONFIG.host,
[SEMATTRS_NET_PEER_PORT]: CONFIG.port,
[SEMATTRS_DB_CONNECTION_STRING]: REDIS_URL,
};

const unsetStatus: SpanStatus = {
Expand All @@ -69,9 +76,8 @@ const predictableStackTrace =
const sanitizeEventForAssertion = (span: ReadableSpan) => {
span.events.forEach(e => {
// stack trace includes data such as /user/{userName}/repos/{projectName}
if (e.attributes?.[SemanticAttributes.EXCEPTION_STACKTRACE]) {
e.attributes[SemanticAttributes.EXCEPTION_STACKTRACE] =
predictableStackTrace;
if (e.attributes?.[SEMATTRS_EXCEPTION_STACKTRACE]) {
e.attributes[SEMATTRS_EXCEPTION_STACKTRACE] = predictableStackTrace;
}

// since time will change on each test invocation, it is being replaced to predicable value
Expand Down Expand Up @@ -134,7 +140,7 @@ describe('ioredis', () => {
let client: ioredisTypes.Redis;
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: 'connect',
[SEMATTRS_DB_STATEMENT]: 'connect',
};
const readyHandler = () => {
const endedSpans = memoryExporter.getFinishedSpans();
Expand Down Expand Up @@ -244,7 +250,7 @@ describe('ioredis', () => {
it(`should create a child span for cb style ${command.description}`, done => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: `${command.name} ${command.expectedDbStatement}`,
[SEMATTRS_DB_STATEMENT]: `${command.name} ${command.expectedDbStatement}`,
};
const span = provider
.getTracer('ioredis-test')
Expand Down Expand Up @@ -274,7 +280,7 @@ describe('ioredis', () => {
it('should create a child span for hset promise', async () => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: `hset ${hashKeyName} random [1 other arguments]`,
[SEMATTRS_DB_STATEMENT]: `hset ${hashKeyName} random [1 other arguments]`,
};
const span = provider.getTracer('ioredis-test').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
Expand Down Expand Up @@ -316,17 +322,15 @@ describe('ioredis', () => {
const exceptionEvent = ioredisSpan.events[0];
assert.strictEqual(exceptionEvent.name, 'exception');
assert.strictEqual(
exceptionEvent.attributes?.[SemanticAttributes.EXCEPTION_MESSAGE],
exceptionEvent.attributes?.[SEMATTRS_EXCEPTION_MESSAGE],
ex.message
);
assert.strictEqual(
exceptionEvent.attributes?.[
SemanticAttributes.EXCEPTION_STACKTRACE
],
exceptionEvent.attributes?.[SEMATTRS_EXCEPTION_STACKTRACE],
ex.stack
);
assert.strictEqual(
exceptionEvent.attributes?.[SemanticAttributes.EXCEPTION_TYPE],
exceptionEvent.attributes?.[SEMATTRS_EXCEPTION_TYPE],
ex.name
);
}
Expand All @@ -336,7 +340,7 @@ describe('ioredis', () => {
it('should create a child span for streamify scanning', done => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000',
[SEMATTRS_DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000',
};
const span = provider.getTracer('ioredis-test').startSpan('test span');
context.with(trace.setSpan(context.active(), span), () => {
Expand Down Expand Up @@ -412,7 +416,7 @@ describe('ioredis', () => {

const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: 'subscribe news music',
[SEMATTRS_DB_STATEMENT]: 'subscribe news music',
};
testUtils.assertSpan(
endedSpans[4],
Expand All @@ -431,7 +435,7 @@ describe('ioredis', () => {
it('should create a child span for multi/transaction', done => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: 'multi',
[SEMATTRS_DB_STATEMENT]: 'multi',
};

const span = provider.getTracer('ioredis-test').startSpan('test span');
Expand Down Expand Up @@ -467,7 +471,7 @@ describe('ioredis', () => {
it('should create a child span for pipeline', done => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: 'set foo [1 other arguments]',
[SEMATTRS_DB_STATEMENT]: 'set foo [1 other arguments]',
};

const span = provider.getTracer('ioredis-test').startSpan('test span');
Expand Down Expand Up @@ -501,7 +505,7 @@ describe('ioredis', () => {
it('should create a child span for get promise', async () => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: `get ${testKeyName}`,
[SEMATTRS_DB_STATEMENT]: `get ${testKeyName}`,
};
const span = provider.getTracer('ioredis-test').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
Expand Down Expand Up @@ -530,7 +534,7 @@ describe('ioredis', () => {
it('should create a child span for del', async () => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: `del ${testKeyName}`,
[SEMATTRS_DB_STATEMENT]: `del ${testKeyName}`,
};
const span = provider.getTracer('ioredis-test').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
Expand Down Expand Up @@ -564,7 +568,7 @@ describe('ioredis', () => {

const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`,
[SEMATTRS_DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`,
};

const span = provider.getTracer('ioredis-test').startSpan('test span');
Expand Down Expand Up @@ -597,11 +601,10 @@ describe('ioredis', () => {
[
{
attributes: {
[SemanticAttributes.EXCEPTION_MESSAGE]:
[SEMATTRS_EXCEPTION_MESSAGE]:
'NOSCRIPT No matching script. Please use EVAL.',
[SemanticAttributes.EXCEPTION_STACKTRACE]:
predictableStackTrace,
[SemanticAttributes.EXCEPTION_TYPE]: 'ReplyError',
[SEMATTRS_EXCEPTION_STACKTRACE]: predictableStackTrace,
[SEMATTRS_EXCEPTION_TYPE]: 'ReplyError',
},
name: 'exception',
time: [0, 0],
Expand Down Expand Up @@ -672,7 +675,7 @@ describe('ioredis', () => {
SpanKind.CLIENT,
{
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`,
[SEMATTRS_DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`,
},
[],
unsetStatus
Expand All @@ -698,7 +701,7 @@ describe('ioredis', () => {
SpanKind.CLIENT,
{
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: 'connect',
[SEMATTRS_DB_STATEMENT]: 'connect',
},
[],
unsetStatus
Expand Down Expand Up @@ -747,7 +750,7 @@ describe('ioredis', () => {
it(`should tag the span with a custom db.statement for cb style ${command.description}`, done => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: dbStatementSerializer(
[SEMATTRS_DB_STATEMENT]: dbStatementSerializer(
command.name,
command.args
),
Expand Down Expand Up @@ -990,7 +993,7 @@ describe('ioredis', () => {
operation.args
);
assert.strictEqual(
endedSpans[0].attributes[SemanticAttributes.DB_STATEMENT],
endedSpans[0].attributes[SEMATTRS_DB_STATEMENT],
expectedStatement
);
done();
Expand Down