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.

Sign up for GitHub

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

Jump to bottom

feat(http): Allow to opt-out of instrumenting incoming/outgoing requests #4643

Merged
merged 5 commits into from
Jul 25, 2024
Merged

feat(http): Allow to opt-out of instrumenting incoming/outgoing requests #4643

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b9581fe
feat(http): Allow to opt-out of instrumenting incoming/outgoing requests
mydea Apr 17, 2024
3d6f8ec
fix tests
mydea Apr 17, 2024
321f07c
PR feedback
mydea Jul 17, 2024
4e8762d
Merge branch 'main' into fn/http-disable-instrumentation
trentm Jul 25, 2024
bb787bf
add a changelog entry
trentm Jul 25, 2024
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
2 changes: 2 additions & 0 deletions experimental/packages/opentelemetry-instrumentation-http/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ Http instrumentation has few options available to choose from. You can set the f
| [`startOutgoingSpanHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L99) | `StartOutgoingSpanCustomAttributeFunction` | Function for adding custom attributes before a span is started in outgoingRequest |
| `ignoreIncomingRequestHook` | `IgnoreIncomingRequestFunction` | Http instrumentation will not trace all incoming requests that matched with custom function |
| `ignoreOutgoingRequestHook` | `IgnoreOutgoingRequestFunction` | Http instrumentation will not trace all outgoing requests that matched with custom function |
| `disableOutgoingRequestInstrumentation` | `boolean` | Set to true to avoid instrumenting outgoing requests at all. This can be helpful when another instrumentation handles outgoing requests. |
| `disableIncomingRequestInstrumentation` | `boolean` | Set to true to avoid instrumenting incoming requests at all. This can be helpful when another instrumentation handles incoming requests. |
| [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L101) | `string` | The primary server name of the matched virtual host. |
| [`requireParentforOutgoingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L103) | Boolean | Require that is a parent span to create new span for outgoing requests. |
| [`requireParentforIncomingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L105) | Boolean | Require that is a parent span to create new span for incoming requests. |
Expand Down
89 changes: 53 additions & 36 deletions experimental/packages/opentelemetry-instrumentation-http/src/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,29 +110,37 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
'http',
['*'],
(moduleExports: Http): Http => {
this._wrap(
moduleExports,
'request',
this._getPatchOutgoingRequestFunction('http')
);
this._wrap(
moduleExports,
'get',
this._getPatchOutgoingGetFunction(moduleExports.request)
);
this._wrap(
moduleExports.Server.prototype,
'emit',
this._getPatchIncomingRequestFunction('http')
);
if (!this.getConfig().disableOutgoingRequestInstrumentation) {
this._wrap(
moduleExports,
'request',
this._getPatchOutgoingRequestFunction('http')
);
this._wrap(
moduleExports,
'get',
this._getPatchOutgoingGetFunction(moduleExports.request)
);
}
if (!this.getConfig().disableIncomingRequestInstrumentation) {
this._wrap(
moduleExports.Server.prototype,
'emit',
this._getPatchIncomingRequestFunction('http')
);
}
return moduleExports;
},
(moduleExports: Http) => {
if (moduleExports === undefined) return;

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
this._unwrap(moduleExports.Server.prototype, 'emit');
if (!this.getConfig().disableOutgoingRequestInstrumentation) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't this be better using the isWrapped check ?

if (isWrapped(module.request)) {
this._unwrap(module, 'request')
}

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
}
if (!this.getConfig().disableIncomingRequestInstrumentation) {
this._unwrap(moduleExports.Server.prototype, 'emit');
}
}
);
}
Expand All @@ -142,29 +150,38 @@ export class HttpInstrumentation extends InstrumentationBase<HttpInstrumentation
'https',
['*'],
(moduleExports: Https): Https => {
this._wrap(
moduleExports,
'request',
this._getPatchHttpsOutgoingRequestFunction('https')
);
this._wrap(
moduleExports,
'get',
this._getPatchHttpsOutgoingGetFunction(moduleExports.request)
);
this._wrap(
moduleExports.Server.prototype,
'emit',
this._getPatchIncomingRequestFunction('https')
);
if (!this.getConfig().disableOutgoingRequestInstrumentation) {
this._wrap(
moduleExports,
'request',
this._getPatchHttpsOutgoingRequestFunction('https')
);

this._wrap(
moduleExports,
'get',
this._getPatchHttpsOutgoingGetFunction(moduleExports.request)
);
}
if (!this.getConfig().disableIncomingRequestInstrumentation) {
this._wrap(
moduleExports.Server.prototype,
'emit',
this._getPatchIncomingRequestFunction('https')
);
}
return moduleExports;
},
(moduleExports: Https) => {
if (moduleExports === undefined) return;

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
this._unwrap(moduleExports.Server.prototype, 'emit');
if (!this.getConfig().disableOutgoingRequestInstrumentation) {
this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
}
if (!this.getConfig().disableIncomingRequestInstrumentation) {
this._unwrap(moduleExports.Server.prototype, 'emit');
}
}
);
}
Expand Down
4 changes: 4 additions & 0 deletions experimental/packages/opentelemetry-instrumentation-http/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ export interface HttpInstrumentationConfig extends InstrumentationConfig {
ignoreOutgoingUrls?: IgnoreMatcher[];
/** Not trace all outgoing requests that matched with custom function */
ignoreOutgoingRequestHook?: IgnoreOutgoingRequestFunction;
/** If set to true, incoming requests will not be instrumented at all. */
disableIncomingRequestInstrumentation?: boolean;
/** If set to true, outgoing requests will not be instrumented at all. */
disableOutgoingRequestInstrumentation?: boolean;
/** Function for adding custom attributes after response is handled */
applyCustomAttributesOnSpan?: HttpCustomAttributeFunction;
/** Function for adding custom attributes before request is handled */
Expand Down
49 changes: 49 additions & 0 deletions ...rimental/packages/opentelemetry-instrumentation-http/test/functionals/http-enable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,55 @@ describe('HttpInstrumentation', () => {
});
});

describe('partially disable instrumentation', () => {
beforeEach(() => {
memoryExporter.reset();
});

afterEach(() => {
server.close();
instrumentation.disable();
});

it('allows to disable outgoing request instrumentation', () => {
server.close();
instrumentation.disable();

instrumentation.setConfig({
disableOutgoingRequestInstrumentation: true,
});
instrumentation.enable();
server = http.createServer((_request, response) => {
response.end('Test Server Response');
});

server.listen(serverPort);

assert.strictEqual(isWrapped(http.Server.prototype.emit), true);
assert.strictEqual(isWrapped(http.get), false);
assert.strictEqual(isWrapped(http.request), false);
});

it('allows to disable incoming request instrumentation', () => {
server.close();
instrumentation.disable();

instrumentation.setConfig({
disableIncomingRequestInstrumentation: true,
});
instrumentation.enable();
server = http.createServer((_request, response) => {
response.end('Test Server Response');
});

server.listen(serverPort);

assert.strictEqual(isWrapped(http.Server.prototype.emit), false);
assert.strictEqual(isWrapped(http.get), true);
assert.strictEqual(isWrapped(http.request), true);
});
});

describe('with good instrumentation options', () => {
beforeEach(() => {
memoryExporter.reset();
Expand Down
56 changes: 56 additions & 0 deletions ...imental/packages/opentelemetry-instrumentation-http/test/functionals/https-enable.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ describe('HttpsInstrumentation', () => {
);
});
});

describe('with good instrumentation options', () => {
beforeEach(() => {
memoryExporter.reset();
Expand Down Expand Up @@ -704,5 +705,60 @@ describe('HttpsInstrumentation', () => {
req.end();
});
});

describe('partially disable instrumentation', () => {
beforeEach(() => {
memoryExporter.reset();
});

afterEach(() => {
server.close();
instrumentation.disable();
});

it('allows to disable outgoing request instrumentation', () => {
instrumentation.setConfig({
disableOutgoingRequestInstrumentation: true,
});
instrumentation.enable();
server = https.createServer(
{
key: fs.readFileSync('test/fixtures/server-key.pem'),
cert: fs.readFileSync('test/fixtures/server-cert.pem'),
},
(request, response) => {
response.end('Test Server Response');
}
);

server.listen(serverPort);

assert.strictEqual(isWrapped(http.Server.prototype.emit), true);
assert.strictEqual(isWrapped(http.get), false);
assert.strictEqual(isWrapped(http.request), false);
});

it('allows to disable incoming request instrumentation', () => {
instrumentation.setConfig({
disableIncomingRequestInstrumentation: true,
});
instrumentation.enable();
server = https.createServer(
{
key: fs.readFileSync('test/fixtures/server-key.pem'),
cert: fs.readFileSync('test/fixtures/server-cert.pem'),
},
(request, response) => {
response.end('Test Server Response');
}
);

server.listen(serverPort);

assert.strictEqual(isWrapped(http.Server.prototype.emit), false);
assert.strictEqual(isWrapped(http.get), true);
assert.strictEqual(isWrapped(http.request), true);
});
});
});
});
20 changes: 10 additions & 10 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.