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
  • Loading branch information
vmarchaud authored Jun 4, 2021
1 parent 6c2378c commit 164f95a
Show file tree
Hide file tree
Showing 13 changed files with 186 additions and 103 deletions.
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
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;
}
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);
}
);
});
});
});
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
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
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
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.