Skip to content

Commit

Permalink
fix: change span names for socket-io (#2495)
Browse files Browse the repository at this point in the history
  • Loading branch information
povilasv authored Oct 23, 2024
1 parent 30e8fc5 commit 86dba74
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 30 deletions.
10 changes: 2 additions & 8 deletions plugins/node/instrumentation-socket.io/src/socket.io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,14 +292,9 @@ export class SocketIoInstrumentation extends InstrumentationBase<SocketIoInstrum
}
const wrappedListener = function (this: any, ...args: any[]) {
const eventName = ev;
const defaultNamespace = '/';
const namespace = this.name || this.adapter?.nsp?.name;
const destination =
namespace === defaultNamespace
? eventName
: `${namespace} ${eventName}`;
const span: Span = self.tracer.startSpan(
`${destination} ${MESSAGINGOPERATIONVALUES_RECEIVE}`,
`${MESSAGINGOPERATIONVALUES_RECEIVE} ${namespace}`,
{
kind: SpanKind.CONSUMER,
attributes: {
Expand Down Expand Up @@ -393,8 +388,7 @@ export class SocketIoInstrumentation extends InstrumentationBase<SocketIoInstrum
namespace;
attributes[SEMATTRS_MESSAGING_DESTINATION] = namespace;
}
const spanRooms = rooms.length ? `[${rooms.join()}]` : '';
const span = self.tracer.startSpan(`${namespace}${spanRooms} send`, {
const span = self.tracer.startSpan(`send ${namespace}`, {
kind: SpanKind.PRODUCER,
attributes,
});
Expand Down
50 changes: 28 additions & 22 deletions plugins/node/instrumentation-socket.io/test/socket.io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
io,
getSocketIoSpans,
expectSpan,
expectSpans,
isV2,
} from './utils';

Expand All @@ -56,7 +57,7 @@ describe('SocketIoInstrumentation', () => {
it('emit is instrumented', () => {
const io = createServerInstance();
io.emit('test');
expectSpan('/ send', span => {
expectSpan('send /', span => {
expect(span.kind).toEqual(SpanKind.PRODUCER);
expect(span.attributes[SEMATTRS_MESSAGING_SYSTEM]).toEqual('socket.io');
expect(span.attributes[SEMATTRS_MESSAGING_DESTINATION_KIND]).toEqual(
Expand Down Expand Up @@ -87,14 +88,14 @@ describe('SocketIoInstrumentation', () => {
} catch (error) {}
if (isV2) {
// only for v2: connect do not throw, but are just ignored
return expectSpan('/ send', span => {
return expectSpan('send /', span => {
expect(span.kind).toEqual(SpanKind.PRODUCER);
expect(span.attributes[SEMATTRS_MESSAGING_SYSTEM]).toEqual(
'socket.io'
);
});
}
expectSpan('/ send', span => {
expectSpan('send /', span => {
expect(span.status.code).toEqual(SpanStatusCode.ERROR);
expect(span.status.message).toEqual(
'"connect" is a reserved event name'
Expand All @@ -105,7 +106,7 @@ describe('SocketIoInstrumentation', () => {
it('send is instrumented', () => {
const io = createServerInstance();
io.send('test');
expectSpan('/ send', span => {
expectSpan('send /', span => {
expect(span.kind).toEqual(SpanKind.PRODUCER);
expect(span.attributes[SEMATTRS_MESSAGING_SYSTEM]).toEqual('socket.io');
expect(span.attributes[SEMATTRS_MESSAGING_DESTINATION_KIND]).toEqual(
Expand All @@ -125,7 +126,7 @@ describe('SocketIoInstrumentation', () => {

const io = createServerInstance();
io.emit('test', 1234);
expectSpan('/ send', span => {
expectSpan('send /', span => {
expect(span.attributes['payload']).toEqual(JSON.stringify([1234]));
});
});
Expand Down Expand Up @@ -164,17 +165,22 @@ describe('SocketIoInstrumentation', () => {
socket.on('test_reply', data => {
client.close();
sio.close();

//trace is created after the listener method is completed
setTimeout(() => {
expectSpan(
'test_reply receive',
span => {
expectSpans(
'receive /',
spans => {
try {
expect(span.kind).toEqual(SpanKind.CONSUMER);
expect(span.attributes[SEMATTRS_MESSAGING_SYSTEM]).toEqual(
'socket.io'
);
expect(span.attributes['payload']).toEqual(
expect(spans[0].kind).toEqual(SpanKind.CONSUMER);
expect(
spans[0].attributes[SEMATTRS_MESSAGING_SYSTEM]
).toEqual('socket.io');
expect(spans[1].kind).toEqual(SpanKind.CONSUMER);
expect(
spans[1].attributes[SEMATTRS_MESSAGING_SYSTEM]
).toEqual('socket.io');
expect(spans[1].attributes['payload']).toEqual(
JSON.stringify([data])
);
done();
Expand Down Expand Up @@ -207,7 +213,7 @@ describe('SocketIoInstrumentation', () => {
sio.on('connection', () => {
//trace is created after the listener method is completed
setTimeout(() => {
expectSpan('connection receive', span => {
expectSpan('receive /', span => {
expect(span.kind).toEqual(SpanKind.CONSUMER);
expect(span.attributes[SEMATTRS_MESSAGING_SYSTEM]).toEqual(
'socket.io'
Expand Down Expand Up @@ -238,7 +244,7 @@ describe('SocketIoInstrumentation', () => {
//trace is created after the listener method is completed
setTimeout(() => {
expectSpan(
'test_reply receive',
'receive /',
span => {
try {
expect(span.kind).toEqual(SpanKind.CONSUMER);
Expand Down Expand Up @@ -291,7 +297,7 @@ describe('SocketIoInstrumentation', () => {
const roomName = 'room';
const sio = createServerInstance();
sio.to(roomName).emit('broadcast', '1234');
expectSpan('/[room] send', span => {
expectSpan('send /', span => {
expect(span.attributes[SEMATTRS_MESSAGING_DESTINATION]).toEqual('/');
expect(
span.attributes[SocketIoInstrumentationAttributes.SOCKET_IO_ROOMS]
Expand All @@ -302,7 +308,7 @@ describe('SocketIoInstrumentation', () => {
it('broadcast to multiple rooms', () => {
const sio = createServerInstance();
sio.to('room1').to('room2').emit('broadcast', '1234');
expectSpan('/[room1,room2] send', span => {
expectSpan('send /', span => {
expect(span.attributes[SEMATTRS_MESSAGING_DESTINATION]).toEqual('/');
expect(
span.attributes[SocketIoInstrumentationAttributes.SOCKET_IO_ROOMS]
Expand All @@ -316,7 +322,7 @@ describe('SocketIoInstrumentation', () => {
const io = createServerInstance();
const namespace = io.of('/testing');
namespace.emit('namespace');
expectSpan('/testing send', span => {
expectSpan('send /testing', span => {
expect(span.attributes[SEMATTRS_MESSAGING_DESTINATION]).toEqual(
'/testing'
);
Expand All @@ -331,7 +337,7 @@ describe('SocketIoInstrumentation', () => {
const io = createServerInstance();
const namespace = io.of('/testing');
namespace.to(roomName).emit('broadcast', '1234');
expectSpan('/testing[room] send', span => {
expectSpan('send /testing', span => {
expect(span.attributes[SEMATTRS_MESSAGING_DESTINATION]).toEqual(
'/testing'
);
Expand All @@ -348,7 +354,7 @@ describe('SocketIoInstrumentation', () => {
const io = createServerInstance();
const namespace = io.of('/testing');
namespace.to('room1').to('room2').emit('broadcast', '1234');
expectSpan('/testing[room1,room2] send', span => {
expectSpan('send /testing', span => {
expect(span.attributes[SEMATTRS_MESSAGING_DESTINATION]).toEqual(
'/testing'
);
Expand Down Expand Up @@ -379,7 +385,7 @@ describe('SocketIoInstrumentation', () => {
//trace is created after the listener method is completed
setTimeout(() => {
expectSpan(
'/testing test_reply receive',
'receive /testing',
span => {
try {
expect(span.kind).toEqual(SpanKind.CONSUMER);
Expand Down Expand Up @@ -421,7 +427,7 @@ describe('SocketIoInstrumentation', () => {
client.close();
sio.close();
expectSpan(
`/[${socket.id}] send`,
'send /',
span => {
try {
expect(span.kind).toEqual(SpanKind.PRODUCER);
Expand Down
12 changes: 12 additions & 0 deletions plugins/node/instrumentation-socket.io/test/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,15 @@ export const expectSpan = (
callback(span);
}
};

export const expectSpans = (
spanNames: string,
callback?: (spans: ReadableSpan[]) => void,
spanCount?: number
) => {
const spans = getSocketIoSpans();
expect(spans.length).toEqual(spanCount || 1);
const foundSpans = spans.filter(span => spanNames.includes(span.name));
expect(foundSpans).toBeDefined();
callback?.(foundSpans);
};

0 comments on commit 86dba74

Please sign in to comment.