From 887e82d34d5a1398cedae285243df22d0f25c205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= <18708370+Flarna@users.noreply.github.com> Date: Mon, 8 May 2023 08:19:50 +0200 Subject: [PATCH 1/3] doc(instrumentation): add limitiations section to readme --- experimental/CHANGELOG.md | 2 + .../opentelemetry-instrumentation/README.md | 82 ++++++------------- 2 files changed, 25 insertions(+), 59 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 02d6ab567a0..3fed26571be 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -21,6 +21,8 @@ All notable changes to experimental packages in this project will be documented ### :books: (Refine Doc) +* doc(instrumentation): add limitiations section to readme [#UpdateMe](/~https://github.com/open-telemetry/opentelemetry-js/pull/UpdateMe) @flarna + ### :house: (Internal) ## 0.38.0 diff --git a/experimental/packages/opentelemetry-instrumentation/README.md b/experimental/packages/opentelemetry-instrumentation/README.md index 26e9f8c6962..009be796f48 100644 --- a/experimental/packages/opentelemetry-instrumentation/README.md +++ b/experimental/packages/opentelemetry-instrumentation/README.md @@ -23,9 +23,9 @@ import { import type * as module_name_to_be_patched from 'module_name_to_be_patched'; -export class MyPlugin extends InstrumentationBase { +export class MyInstrumentation extends InstrumentationBase { constructor(config: InstrumentationConfig = {}) { - super('MyPlugin', VERSION, config); + super('MyInstrumentation', VERSION, config); } /** @@ -106,12 +106,12 @@ export class MyPlugin extends InstrumentationBase { } } -// Later +// Later, but before the module to instrument is required -const myPLugin = new MyPlugin(); -myPLugin.setTracerProvider(provider); // this is optional, only if global TracerProvider shouldn't be used -myPLugin.setMeterProvider(meterProvider); // this is optional -myPLugin.enable(); +const myInstrumentationn = new MyInstrumentation(); +myInstrumentation.setTracerProvider(provider); // this is optional, only if global TracerProvider shouldn't be used +myInstrumentation.setMeterProvider(meterProvider); // this is optional +myInstrumentation.enable(); // or use Auto Loader ``` @@ -125,9 +125,9 @@ import { import { Instrumentation } from '@opentelemetry/instrumentation'; -export class MyPlugin extends InstrumentationBase { +export class MyInstrumentation extends InstrumentationBase { constructor(config: InstrumentationConfig = {}) { - super('MyPlugin', VERSION, config); + super('MyInstrumentation', VERSION, config); } private _patchOpen() { @@ -150,59 +150,15 @@ export class MyPlugin extends InstrumentationBase { // Later -const myPLugin = new MyPlugin(); -myPLugin.setTracerProvider(provider); // this is optional, only if global TracerProvider shouldn't be used -myPLugin.setMeterProvider(meterProvider); // this is optional, only if global MeterProvider shouldn't be used -myPLugin.enable(); +const myInstrumentation = new MyInstrumentation(); +myInstrumentation.setTracerProvider(provider); // this is optional, only if global TracerProvider shouldn't be used +myInstrumentation.setMeterProvider(meterProvider); // this is optional, only if global MeterProvider shouldn't be used +myInstrumentation.enable(); // or use Auto Loader ``` ## AutoLoader -Successor of loading plugins through TracerProvider "plugins" option. -It also supersedes PluginLoader for node. The old configurations usually looks like - -### NODE - old way using TracerProvider - not available anymore - -```javascript -const { NodeTracerProvider } = require('@opentelemetry/sdk-trace-node'); -const { B3Propagator } = require('@opentelemetry/propagator-b3'); -const provider = new NodeTracerProvider({ - plugins: { - http: { enabled: false }, - }, -}); -provider.register({ - propagator: new B3Propagator(), -}); -``` - -### WEB - old way using TracerProvider - not available anymore - -```javascript -const { WebTracerProvider } = require('@opentelemetry/sdk-trace-web'); -const { UserInteractionPlugin } = require('@opentelemetry/plugin-user-interaction'); -const { XMLHttpRequestInstrumentation } = require('@opentelemetry/instrumentation-xml-http-request'); -const { B3Propagator } = require('@opentelemetry/propagator-b3'); -const provider = new WebTracerProvider({ - plugins: [ - new UserInteractionPlugin(), - new XMLHttpRequestInstrumentation({ - ignoreUrls: [/localhost/], - propagateTraceHeaderCorsUrls: [ - 'http://localhost:8090', - ], - }), - ], -}); -provider.register({ - propagator: new B3Propagator(), -}); -``` - -After change it will look like this - mixing plugins and instrumentations together -All plugins will be bound to TracerProvider as well as instrumentations - ### NODE - Auto Loader ```javascript @@ -263,14 +219,22 @@ If nothing is specified the global registered provider is used. Usually this is There might be usecase where someone has the need for more providers within an application. Please note that special care must be takes in such setups to avoid leaking information from one provider to the other because there are a lot places where e.g. the global `ContextManager` or `Propagator` is used. +## Limitations + +Instrumentations for external modules (e.g. express, mongodb,...) hook the `require` call. Therefore following conditions need to be met that this mechanism can work: + +* `require` is used. ECMA script modules (using `import`) is not supported as of now +* Instrumentations are registered **before** the module to instrument is `require`ed +* modules are not included in a bundle. Tools like `esbuild`, `webpack`, ... usually have some mechanism to exclude specific modules from bundling + ## License Apache 2.0 - See [LICENSE][license-url] for more information. ## Useful links -- For more information on OpenTelemetry, visit: -- For help or feedback on this project, join us in [GitHub Discussions][discussions-url] +* For more information on OpenTelemetry, visit: +* For help or feedback on this project, join us in [GitHub Discussions][discussions-url] [discussions-url]: /~https://github.com/open-telemetry/opentelemetry-js/discussions [license-url]: /~https://github.com/open-telemetry/opentelemetry-js/blob/main/LICENSE From 7baf5aba982be46c496da56d7728c4071c7a5631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= Date: Mon, 8 May 2023 08:26:39 +0200 Subject: [PATCH 2/3] Update experimental/CHANGELOG.md --- experimental/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 3fed26571be..835b46c4db2 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -21,7 +21,7 @@ All notable changes to experimental packages in this project will be documented ### :books: (Refine Doc) -* doc(instrumentation): add limitiations section to readme [#UpdateMe](/~https://github.com/open-telemetry/opentelemetry-js/pull/UpdateMe) @flarna +* doc(instrumentation): add limitiations section to readme [#3786](/~https://github.com/open-telemetry/opentelemetry-js/pull/3786) @flarna ### :house: (Internal) From 7d1d79565869c43553a72b3bd462a3e69167b189 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerhard=20St=C3=B6bich?= Date: Tue, 9 May 2023 07:51:47 +0200 Subject: [PATCH 3/3] typo --- experimental/packages/opentelemetry-instrumentation/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation/README.md b/experimental/packages/opentelemetry-instrumentation/README.md index 009be796f48..682fc5741a0 100644 --- a/experimental/packages/opentelemetry-instrumentation/README.md +++ b/experimental/packages/opentelemetry-instrumentation/README.md @@ -221,7 +221,7 @@ to avoid leaking information from one provider to the other because there are a ## Limitations -Instrumentations for external modules (e.g. express, mongodb,...) hook the `require` call. Therefore following conditions need to be met that this mechanism can work: +Instrumentations for external modules (e.g. express, mongodb,...) hooks the `require` call. Therefore following conditions need to be met that this mechanism can work: * `require` is used. ECMA script modules (using `import`) is not supported as of now * Instrumentations are registered **before** the module to instrument is `require`ed