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!(instrumentation): add patch and unpatch diag log messages #4641

Merged
merged 6 commits into from
Apr 19, 2024
Merged

feat!(instrumentation): add patch and unpatch diag log messages #4641

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7e6eaf4
refactor(@opentelemetry/instrumentation): add patch and unpatch diag …
blumamir Apr 17, 2024
4f4f54b
Merge remote-tracking branch 'upstream/main' into patch-log
blumamir Apr 17, 2024
ed76808
chore: CHANGELOG
blumamir Apr 17, 2024
38b9a01
fix: exclude version in core packages
blumamir Apr 17, 2024
e824c29
Merge branch 'main' into patch-log
pichlermarc Apr 18, 2024
409cf0b
Merge branch 'main' into patch-log
pichlermarc Apr 19, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/

### :boom: Breaking Change

* feat(instrumentation): add patch and unpatch diag log messages [#4641](https://github.com/open-telemetry/opentelemetry-js/pull/4641)
* Instrumentations should not log patch and unpatch messages to diag channel.

### :rocket: (Enhancement)

* feat(sdk-trace-base): log resource attributes in ConsoleSpanExporter [#4605](https://github.com/open-telemetry/opentelemetry-js/pull/4605) @pichlermarc
Expand Down
6 changes: 2 additions & 4 deletions experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,7 @@ export class GrpcInstrumentation extends InstrumentationBase {
new InstrumentationNodeModuleDefinition<any>(
'@grpc/grpc-js',
['1.*'],
(moduleExports, version) => {
this._diag.debug(`Applying patch for @grpc/grpc-js@${version}`);
moduleExports => {
if (isWrapped(moduleExports.Server.prototype.register)) {
this._unwrap(moduleExports.Server.prototype, 'register');
}
Expand Down Expand Up @@ -174,9 +173,8 @@ export class GrpcInstrumentation extends InstrumentationBase {
);
return moduleExports;
},
(moduleExports, version) => {
moduleExports => {
if (moduleExports === undefined) return;
this._diag.debug(`Removing patch for @grpc/grpc-js@${version}`);

this._unwrap(moduleExports.Server.prototype, 'register');
this._unwrap(moduleExports, 'makeClientConstructor');
Expand Down
6 changes: 0 additions & 6 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 @@ -111,12 +111,10 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
}

private _getHttpInstrumentation() {
const version = process.versions.node;
return new InstrumentationNodeModuleDefinition<Http>(
'http',
['*'],
moduleExports => {
this._diag.debug(`Applying patch for http@${version}`);
if (isWrapped(moduleExports.request)) {
this._unwrap(moduleExports, 'request');
}
Expand Down Expand Up @@ -145,7 +143,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
},
moduleExports => {
if (moduleExports === undefined) return;
this._diag.debug(`Removing patch for http@${version}`);

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
Expand All @@ -155,12 +152,10 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
}

private _getHttpsInstrumentation() {
const version = process.versions.node;
return new InstrumentationNodeModuleDefinition<Https>(
'https',
['*'],
moduleExports => {
this._diag.debug(`Applying patch for https@${version}`);
if (isWrapped(moduleExports.request)) {
this._unwrap(moduleExports, 'request');
}
Expand Down Expand Up @@ -189,7 +184,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
},
moduleExports => {
if (moduleExports === undefined) return;
this._diag.debug(`Removing patch for https@${version}`);

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
Expand Down
54 changes: 54 additions & 0 deletions experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,13 @@ export abstract class InstrumentationBase<T = any>
if (typeof module.patch === 'function') {
module.moduleExports = exports;
if (this._enabled) {
this._diag.debug(
'Applying instrumentation patch for nodejs core module on require hook',
{
module: module.name,
version: module.moduleVersion,
}
);
return module.patch(exports);
}
}
Expand All @@ -199,6 +206,14 @@ export abstract class InstrumentationBase<T = any>
if (typeof module.patch === 'function') {
module.moduleExports = exports;
if (this._enabled) {
this._diag.debug(
'Applying instrumentation patch for module on require hook',
{
module: module.name,
version: module.moduleVersion,
baseDir,
}
);
return module.patch(exports, module.moduleVersion);
}
}
Expand All @@ -216,6 +231,15 @@ export abstract class InstrumentationBase<T = any>
return supportedFileInstrumentations.reduce<T>((patchedExports, file) => {
file.moduleExports = patchedExports;
if (this._enabled) {
this._diag.debug(
'Applying instrumentation patch for nodejs module file on require hook',
{
module: module.name,
version: module.moduleVersion,
fileName: file.name,
baseDir,
}
);
return file.patch(patchedExports, module.moduleVersion);
}
return patchedExports;
Expand All @@ -232,10 +256,25 @@ export abstract class InstrumentationBase<T = any>
if (this._hooks.length > 0) {
for (const module of this._modules) {
if (typeof module.patch === 'function' && module.moduleExports) {
this._diag.debug(
'Applying instrumentation patch for nodejs module on instrumentation enabled',
{
module: module.name,
version: module.moduleVersion,
}
);
module.patch(module.moduleExports, module.moduleVersion);
}
for (const file of module.files) {
if (file.moduleExports) {
this._diag.debug(
'Applying instrumentation patch for nodejs module file on instrumentation enabled',
{
module: module.name,
version: module.moduleVersion,
fileName: file.name,
}
);
file.patch(file.moduleExports, module.moduleVersion);
}
}
Expand Down Expand Up @@ -288,10 +327,25 @@ export abstract class InstrumentationBase<T = any>

for (const module of this._modules) {
if (typeof module.unpatch === 'function' && module.moduleExports) {
this._diag.debug(
'Removing instrumentation patch for nodejs module on instrumentation disabled',
{
module: module.name,
version: module.moduleVersion,
}
);
module.unpatch(module.moduleExports, module.moduleVersion);
}
for (const file of module.files) {
if (file.moduleExports) {
this._diag.debug(
'Removing instrumentation patch for nodejs module file on instrumentation disabled',
{
module: module.name,
version: module.moduleVersion,
fileName: file.name,
}
);
file.unpatch(file.moduleExports, module.moduleVersion);
}
}
Expand Down