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

feat(instrumentation-express): Add support for Express v5 #2437

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -61,41 +61,63 @@
'express',
['>=4.0.0 <5'],
moduleExports => {
const routerProto = moduleExports.Router as unknown as express.Router;
// patch express.Router.route
if (isWrapped(routerProto.route)) {
this._unwrap(routerProto, 'route');
}
this._wrap(routerProto, 'route', this._getRoutePatch());
// patch express.Router.use
if (isWrapped(routerProto.use)) {
this._unwrap(routerProto, 'use');
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this._wrap(routerProto, 'use', this._getRouterUsePatch() as any);
// patch express.Application.use
if (isWrapped(moduleExports.application.use)) {
this._unwrap(moduleExports.application, 'use');
}
this._wrap(
moduleExports.application,
'use',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this._getAppUsePatch() as any
);
return moduleExports;
this._setup(moduleExports, false);
},
moduleExports => {
this._tearDown(moduleExports, false);
}
),
new InstrumentationNodeModuleDefinition(
'express',
['>=5 <6'],
moduleExports => {
this._setup(moduleExports, true);

Check warning on line 74 in plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

View check run for this annotation

Codecov / codecov/patch

plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts#L73-L74

Added lines #L73 - L74 were not covered by tests
},
moduleExports => {
if (moduleExports === undefined) return;
const routerProto = moduleExports.Router as unknown as express.Router;
this._unwrap(routerProto, 'route');
this._unwrap(routerProto, 'use');
this._unwrap(moduleExports.application, 'use');
this._tearDown(moduleExports, true);

Check warning on line 77 in plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

View check run for this annotation

Codecov / codecov/patch

plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts#L77

Added line #L77 was not covered by tests
}
),
];
}

private _setup(moduleExports: any, isExpressV5: boolean) {
const routerProto = isExpressV5
? moduleExports.Router.prototype

Check warning on line 85 in plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

View check run for this annotation

Codecov / codecov/patch

plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts#L85

Added line #L85 was not covered by tests
: moduleExports.Router;
// patch express.Router.route
if (isWrapped(routerProto.route)) {
this._unwrap(routerProto, 'route');
}
this._wrap(routerProto, 'route', this._getRoutePatch());
// patch express.Router.use
if (isWrapped(routerProto.use)) {
this._unwrap(routerProto, 'use');
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this._wrap(routerProto, 'use', this._getRouterUsePatch() as any);
// patch express.Application.use
if (isWrapped(moduleExports.application.use)) {
this._unwrap(moduleExports.application, 'use');
}
this._wrap(
moduleExports.application,
'use',
// eslint-disable-next-line @typescript-eslint/no-explicit-any
this._getAppUsePatch(isExpressV5) as any
);
return moduleExports;
}

private _tearDown(moduleExports: any, isExpressV5: boolean) {
if (moduleExports === undefined) return;
const routerProto = isExpressV5
? moduleExports.Router.prototype

Check warning on line 114 in plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts

View check run for this annotation

Codecov / codecov/patch

plugins/node/opentelemetry-instrumentation-express/src/instrumentation.ts#L114

Added line #L114 was not covered by tests
: moduleExports.Router;
this._unwrap(routerProto, 'route');
this._unwrap(routerProto, 'use');
this._unwrap(moduleExports.application, 'use');
}

/**
* Get the patch for Router.route function
*/
Expand Down Expand Up @@ -135,16 +157,22 @@
/**
* Get the patch for Application.use function
*/
private _getAppUsePatch() {
private _getAppUsePatch(isExpressV5: boolean) {
const instrumentation = this;
return function (original: express.Application['use']) {
return function use(
this: { _router: ExpressRouter },
// In express 5.x the router is stored in `router` whereas in 4.x it's stored in `_router`
this: { _router?: ExpressRouter; router?: ExpressRouter },
...args: Parameters<typeof original>
) {
// if we access app.router in express 4.x we trigger an assertion error
// This property existed in v3, was removed in v4 and then re-added in v5
const router = isExpressV5 ? this.router : this._router;
const route = original.apply(this, args);
const layer = this._router.stack[this._router.stack.length - 1];
instrumentation._applyPatch(layer, getLayerPath(args));
if (router) {
const layer = router.stack[router.stack.length - 1];
instrumentation._applyPatch(layer, getLayerPath(args));
}
return route;
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
*/

import type { Request } from 'express';
import { Attributes } from '@opentelemetry/api';

/**
* This symbol is used to mark express layer as being already instrumented
Expand Down Expand Up @@ -48,11 +47,6 @@ export type PathParams = string | RegExp | Array<string | RegExp>;

// https://github.com/expressjs/express/blob/main/lib/router/index.js#L53
export type ExpressRouter = {
params: { [key: string]: string };
_params: string[];
caseSensitive: boolean;
mergeParams: boolean;
strict: boolean;
stack: ExpressLayer[];
};

Expand All @@ -61,13 +55,6 @@ export type ExpressLayer = {
handle: Function & Record<string, any>;
[kLayerPatched]?: boolean;
name: string;
params: { [key: string]: string };
path: string;
regexp: RegExp;
route?: ExpressLayer;
};

export type LayerMetadata = {
attributes: Attributes;
name: string;
};
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ export const getLayerMetadata = (
},
name: `router - ${extractedRouterPath}`,
};
} else if (layer.name === 'bound dispatch') {
} else if (layer.name === 'bound dispatch' || layer.name === 'handle') {
return {
attributes: {
[AttributeNames.EXPRESS_NAME]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ import {
import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
import * as assert from 'assert';
import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
import { ExpressLayerType } from '../src/enums/ExpressLayerType';
import { AttributeNames } from '../src/enums/AttributeNames';
import { ExpressInstrumentation, ExpressInstrumentationConfig } from '../src';
import { ExpressLayerType } from '../../src/enums/ExpressLayerType';
import { AttributeNames } from '../../src/enums/AttributeNames';
import {
ExpressInstrumentation,
ExpressInstrumentationConfig,
} from '../../src';
import { createServer, httpRequest } from './utils';

const instrumentation = new ExpressInstrumentation({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import {
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import { AttributeNames } from '../src/enums/AttributeNames';
import { ExpressInstrumentation } from '../src';
import { AttributeNames } from '../../src/enums/AttributeNames';
import { ExpressInstrumentation } from '../../src';
import { createServer, httpRequest, serverWithMiddleware } from './utils';
import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions';
import * as testUtils from '@opentelemetry/contrib-test-utils';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { promisify } from 'util';
import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';

import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { ExpressInstrumentation } from '../../build/src/index.js';
import { ExpressInstrumentation } from '../../../build/src/index.js';

const sdk = createTestNodeSdk({
serviceName: 'use-express-nested',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { promisify } from 'util';
import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';

import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { ExpressInstrumentation } from '../../build/src/index.js';
import { ExpressInstrumentation } from '../../../build/src/index.js';

const sdk = createTestNodeSdk({
serviceName: 'use-express-regex',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { promisify } from 'util';
import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';

import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { ExpressInstrumentation } from '../../build/src/index.js';
import { ExpressInstrumentation } from '../../../build/src/index.js';

const sdk = createTestNodeSdk({
serviceName: 'use-express-nested',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,12 @@ import { promisify } from 'util';
import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';

import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { ExpressInstrumentation } from '../../build/src/index.js';
import { ExpressInstrumentation } from '../../../build/src/index.js';

const sdk = createTestNodeSdk({
serviceName: 'use-express',
instrumentations: [
new HttpInstrumentation(),
new ExpressInstrumentation()
]
})
instrumentations: [new HttpInstrumentation(), new ExpressInstrumentation()],
});
sdk.start();

import express from 'express';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import {
import * as assert from 'assert';
import type * as http from 'http';
import * as sinon from 'sinon';
import { ExpressInstrumentation } from '../src';
import { ExpressRequestInfo, SpanNameHook } from '../src/types';
import { ExpressLayerType } from '../src/enums/ExpressLayerType';
import { ExpressInstrumentation } from '../../src';
import { ExpressRequestInfo, SpanNameHook } from '../../src/types';
import { ExpressLayerType } from '../../src/enums/ExpressLayerType';
import { SEMATTRS_HTTP_METHOD } from '@opentelemetry/semantic-conventions';

const instrumentation = new ExpressInstrumentation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import {
} from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import { RPCMetadata, RPCType, setRPCMetadata } from '@opentelemetry/core';
import { AttributeNames } from '../src/enums/AttributeNames';
import { ExpressInstrumentation, ExpressLayerType } from '../src';
import { AttributeNames } from '../../src/enums/AttributeNames';
import { ExpressInstrumentation, ExpressLayerType } from '../../src';
import { createServer, httpRequest } from './utils';

const instrumentation = new ExpressInstrumentation({
Expand Down
Loading