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.

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

fix: change span names for socket-io #2495

Merged
merged 1 commit into from
Oct 23, 2024
Merged
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: 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);
};