Skip to content

Commit

Permalink
feat: use rpcMetadata to update http span name open-telemetry#464 (op…
Browse files Browse the repository at this point in the history 
…en-telemetry#517)
  • Loading branch information
@vmarchaud
vmarchaud committed Jun 4, 2021
1 parent 6c2378c commit 164f95a
Show file tree
Hide file tree
Showing 13 changed files with 186 additions and 103 deletions.
35 changes: 21 additions & 14 deletions plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
* limitations under the License.
*/

import { hrTime } from '@opentelemetry/core';
import {
hrTime,
setRPCMetadata,
getRPCMetadata,
RPCType,
} from '@opentelemetry/core';
import { trace, context, diag, SpanAttributes } from '@opentelemetry/api';
import * as express from 'express';
import {
Expand All @@ -23,7 +28,6 @@ import {
PatchedRequest,
_LAYERS_STORE_PROPERTY,
ExpressInstrumentationConfig,
ExpressInstrumentationSpan,
} from './types';
import { ExpressLayerType } from './enums/ExpressLayerType';
import { AttributeNames } from './enums/AttributeNames';
Expand Down Expand Up @@ -183,7 +187,7 @@ export class ExpressInstrumentation extends InstrumentationBase<
.filter(path => path !== '/' && path !== '/*')
.join('');
const attributes: SpanAttributes = {
[SemanticAttributes.HTTP_ROUTE]: route.length > 0 ? route : undefined,
[SemanticAttributes.HTTP_ROUTE]: route.length > 0 ? route : '/',
};
const metadata = getLayerMetadata(layer, layerPath);
const type = metadata.attributes[
Expand All @@ -192,19 +196,15 @@ export class ExpressInstrumentation extends InstrumentationBase<

// Rename the root http span in case we haven't done it already
// once we reach the request handler
const rpcMetadata = getRPCMetadata(context.active());
if (
metadata.attributes[AttributeNames.EXPRESS_TYPE] ===
ExpressLayerType.REQUEST_HANDLER
ExpressLayerType.REQUEST_HANDLER &&
rpcMetadata?.type === RPCType.HTTP
) {
const parent = trace.getSpan(
context.active()
) as ExpressInstrumentationSpan;
if (parent?.name) {
const parentRoute = parent.name.split(' ')[1];
if (!route.includes(parentRoute)) {
parent.updateName(`${req.method} ${route}`);
}
}
rpcMetadata.span.updateName(
`${req.method} ${route.length > 0 ? route : '/'}`
);
}

// verify against the config if the layer should be ignored
Expand Down Expand Up @@ -242,6 +242,13 @@ export class ExpressInstrumentation extends InstrumentationBase<
// verify we have a callback
const args = Array.from(arguments);
const callbackIdx = args.findIndex(arg => typeof arg === 'function');
const newContext =
rpcMetadata?.type === RPCType.HTTP
? setRPCMetadata(
context.active(),
Object.assign(rpcMetadata, { route: route })
)
: context.active();
if (callbackIdx >= 0) {
arguments[callbackIdx] = function () {
if (spanHasEnded === false) {
Expand All @@ -253,7 +260,7 @@ export class ExpressInstrumentation extends InstrumentationBase<
(req[_LAYERS_STORE_PROPERTY] as string[]).pop();
}
const callback = args[callbackIdx] as Function;
return context.bind(callback).apply(this, arguments);
return context.bind(callback, newContext).apply(this, arguments);
};
}
const result = original.apply(this, arguments);
Expand Down
9 changes: 1 addition & 8 deletions plugins/node/opentelemetry-instrumentation-express/src/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { kLayerPatched } from './';
import { Request } from 'express';
import { SpanAttributes, Span } from '@opentelemetry/api';
import { SpanAttributes } from '@opentelemetry/api';
import { InstrumentationConfig } from '@opentelemetry/instrumentation';
import { ExpressLayerType } from './enums/ExpressLayerType';

Expand Down Expand Up @@ -79,10 +79,3 @@ export interface ExpressInstrumentationConfig extends InstrumentationConfig {
/** Ignore specific layers based on their type */
ignoreLayersType?: ExpressLayerType[];
}

/**
* extends opentelemetry/api Span object to instrument the root span name of http instrumentation
*/
export interface ExpressInstrumentationSpan extends Span {
name?: string;
}
76 changes: 66 additions & 10 deletions plugins/node/opentelemetry-instrumentation-express/test/custom-config.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ import {
} from '@opentelemetry/tracing';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import * as assert from 'assert';
import { ExpressInstrumentationSpan } from '../src/types';
import { RPCType, setRPCMetadata } from '@opentelemetry/core';
import { ExpressLayerType } from '../src/enums/ExpressLayerType';
import { AttributeNames } from '../src/enums/AttributeNames';
import { ExpressInstrumentation } from '../src';
import { ExpressInstrumentation, ExpressInstrumentationConfig } from '../src';

const instrumentation = new ExpressInstrumentation({
ignoreLayersType: [ExpressLayerType.MIDDLEWARE],
Expand Down Expand Up @@ -129,9 +129,16 @@ describe('ExpressInstrumentation', () => {
});

it('should not repeat middleware paths in the span name', async () => {
app.use((req, res, next) =>
context.with(trace.setSpan(context.active(), rootSpan), next)
);
app.use((req, res, next) => {
const rpcMetadata = { type: RPCType.HTTP, span: rootSpan };
return context.with(
setRPCMetadata(
trace.setSpan(context.active(), rootSpan),
rpcMetadata
),
next
);
});

app.use('/mw', (req, res, next) => {
next();
Expand All @@ -141,9 +148,7 @@ describe('ExpressInstrumentation', () => {
res.send('ok');
});

const rootSpan = tracer.startSpan(
'rootSpan'
) as ExpressInstrumentationSpan;
const rootSpan = tracer.startSpan('rootSpan');
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);

await context.with(
Expand All @@ -153,8 +158,6 @@ describe('ExpressInstrumentation', () => {
assert.strictEqual(response, 'ok');
rootSpan.end();

assert.strictEqual(rootSpan.name, 'GET /mw');

const spans = memoryExporter.getFinishedSpans();

const requestHandlerSpan = memoryExporter
Expand All @@ -175,5 +178,58 @@ describe('ExpressInstrumentation', () => {
}
);
});

it('should correctly name http root path when its /', async () => {
instrumentation.setConfig({
ignoreLayerTypes: [
ExpressLayerType.MIDDLEWARE,
ExpressLayerType.REQUEST_HANDLER,
],
} as ExpressInstrumentationConfig);
app.use((req, res, next) => {
const rpcMetadata = { type: RPCType.HTTP, span: rootSpan };
return context.with(
setRPCMetadata(
trace.setSpan(context.active(), rootSpan),
rpcMetadata
),
next
);
});

app.get('/', (req, res) => {
res.send('ok');
});

const rootSpan = tracer.startSpan('rootSpan');
assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);

await context.with(
trace.setSpan(context.active(), rootSpan),
async () => {
const response = await httpRequest.get(`http://localhost:${port}/`);
assert.strictEqual(response, 'ok');
rootSpan.end();

const spans = memoryExporter.getFinishedSpans();

const requestHandlerSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name.includes('request handler'));
assert.notStrictEqual(requestHandlerSpan, undefined);
assert.strictEqual(
requestHandlerSpan?.attributes[SemanticAttributes.HTTP_ROUTE],
'/'
);

assert.strictEqual(
requestHandlerSpan?.attributes[AttributeNames.EXPRESS_TYPE],
'request_handler'
);
const exportedRootSpan = spans.find(span => span.name === 'GET /');
assert.notStrictEqual(exportedRootSpan, undefined);
}
);
});
});
});
17 changes: 9 additions & 8 deletions plugins/node/opentelemetry-instrumentation-express/test/express.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
SimpleSpanProcessor,
} from '@opentelemetry/tracing';
import * as assert from 'assert';
import { ExpressInstrumentationSpan } from '../src/types';
import { setRPCMetadata, RPCType } from '@opentelemetry/core';
import { AttributeNames } from '../src/enums/AttributeNames';
import { ExpressInstrumentation } from '../src';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
Expand Down Expand Up @@ -61,9 +61,13 @@ const serverWithMiddleware = async (
): Promise<http.Server> => {
const app = express();
if (tracer) {
app.use((req, res, next) =>
context.with(trace.setSpan(context.active(), rootSpan), next)
);
app.use((req, res, next) => {
const rpcMetadata = { type: RPCType.HTTP, span: rootSpan };
return context.with(
setRPCMetadata(trace.setSpan(context.active(), rootSpan), rpcMetadata),
next
);
});
}

app.use(express.json());
Expand Down Expand Up @@ -109,9 +113,7 @@ describe('ExpressInstrumentation', () => {

describe('Instrumenting normal get operations', () => {
it('should create a child span for middlewares', async () => {
const rootSpan = tracer.startSpan(
'rootSpan'
) as ExpressInstrumentationSpan;
const rootSpan = tracer.startSpan('rootSpan');
const app = express();
app.use((req, res, next) =>
context.with(trace.setSpan(context.active(), rootSpan), next)
Expand Down Expand Up @@ -148,7 +150,6 @@ describe('ExpressInstrumentation', () => {
);
assert.strictEqual(response, 'tata');
rootSpan.end();
assert.strictEqual(rootSpan.name, 'GET /toto/:id');
assert.strictEqual(finishListenerCount, 2);
assert.notStrictEqual(
memoryExporter
Expand Down
23 changes: 15 additions & 8 deletions plugins/node/opentelemetry-instrumentation-express/test/ignore-all.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
* limitations under the License.
*/

import { context, trace } from '@opentelemetry/api';
import { context, trace, Span } from '@opentelemetry/api';
import { NodeTracerProvider } from '@opentelemetry/node';
import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
import {
InMemorySpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/tracing';
import * as assert from 'assert';
import { ExpressInstrumentationSpan } from '../src/types';
import { RPCType, setRPCMetadata } from '@opentelemetry/core';
import { AttributeNames } from '../src/enums/AttributeNames';
import { ExpressInstrumentation, ExpressLayerType } from '../src';

Expand Down Expand Up @@ -81,14 +81,22 @@ describe('ExpressInstrumentation', () => {

describe('when route exists', () => {
let server: http.Server;
let rootSpan: ExpressInstrumentationSpan;
let rootSpan: Span;

beforeEach(async () => {
rootSpan = tracer.startSpan('rootSpan') as ExpressInstrumentationSpan;
rootSpan = tracer.startSpan('rootSpan');
const app = express();
app.use((req, res, next) =>
context.with(trace.setSpan(context.active(), rootSpan), next)
);

app.use((req, res, next) => {
const rpcMetadata = { type: RPCType.HTTP, span: rootSpan };
return context.with(
setRPCMetadata(
trace.setSpan(context.active(), rootSpan),
rpcMetadata
),
next
);
});
app.use(express.json());
app.use((req, res, next) => {
for (let i = 0; i < 1000; i++) {}
Expand Down Expand Up @@ -144,7 +152,6 @@ describe('ExpressInstrumentation', () => {
async () => {
await httpRequest.get(`http://localhost:${port}/toto/tata`);
rootSpan.end();
assert.strictEqual(rootSpan.name, 'GET /toto/:id');
const exportedRootSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name === 'GET /toto/:id');
Expand Down
1 change: 1 addition & 0 deletions plugins/node/opentelemetry-instrumentation-koa/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
},
"dependencies": {
"@opentelemetry/api": "^0.20.0",
"@opentelemetry/core": "^0.20.0",
"@opentelemetry/instrumentation": "^0.20.0",
"@opentelemetry/semantic-conventions": "^0.20.0",
"@types/koa": "2.13.1",
Expand Down
52 changes: 23 additions & 29 deletions plugins/node/opentelemetry-instrumentation-koa/src/instrumentation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
import { AttributeNames } from './enums/AttributeNames';
import { VERSION } from './version';
import { getMiddlewareMetadata } from './utils';
import { getRPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';

/** Koa instrumentation for OpenTelemetry */
export class KoaInstrumentation extends InstrumentationBase<typeof koa> {
Expand Down Expand Up @@ -143,41 +144,34 @@ export class KoaInstrumentation extends InstrumentationBase<typeof koa> {
attributes: metadata.attributes,
});

if (!context.request.ctx.parentSpan) {
context.request.ctx.parentSpan = parent;
}
const rpcMetadata = getRPCMetadata(api.context.active());

if (
metadata.attributes[AttributeNames.KOA_TYPE] === KoaLayerType.ROUTER
metadata.attributes[AttributeNames.KOA_TYPE] === KoaLayerType.ROUTER &&
rpcMetadata?.type === RPCType.HTTP
) {
if (context.request.ctx.parentSpan.name) {
const parentRoute = context.request.ctx.parentSpan.name.split(' ')[1];
if (
context._matchedRoute &&
!context._matchedRoute.toString().includes(parentRoute)
) {
context.request.ctx.parentSpan.updateName(
`${context.method} ${context._matchedRoute}`
);

delete context.request.ctx.parentSpan;
}
}
rpcMetadata.span.updateName(
`${context.method} ${context._matchedRoute}`
);
}

return api.context.with(
api.trace.setSpan(api.context.active(), span),
async () => {
try {
return await middlewareLayer(context, next);
} catch (err) {
span.recordException(err);
throw err;
} finally {
span.end();
}
let newContext = api.trace.setSpan(api.context.active(), span);
if (rpcMetadata?.type === RPCType.HTTP) {
newContext = setRPCMetadata(
newContext,
Object.assign(rpcMetadata, { route: context._matchedRoute })
);
}
return api.context.with(newContext, async () => {
try {
return await middlewareLayer(context, next);
} catch (err) {
span.recordException(err);
throw err;
} finally {
span.end();
}
);
});
};
}
}
Loading

0 comments on commit 164f95a

Please sign in to comment.