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: ESM support for instrumentation #3698

Merged
merged 46 commits into from
May 15, 2023
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
43fd59e
docs: add example for esm-http-ts
JamieDanielson Mar 21, 2023
e5215fe
fix: use instr version not node version
JamieDanielson Mar 21, 2023
3c51c6d
deps: add import-in-the-middle
JamieDanielson Mar 21, 2023
8444af3
feat: add iitm for esm instr
JamieDanielson Mar 21, 2023
e4478d9
docs: add readme notes from valentin
JamieDanielson Mar 21, 2023
1434b27
wip: add test based on valentin's test
JamieDanielson Mar 21, 2023
a533f9f
remove changes to ritm init
JamieDanielson Mar 22, 2023
a43089b
update example - use build dir, add readme
JamieDanielson Mar 22, 2023
85a2e8f
add changelog entry
JamieDanielson Mar 22, 2023
e0b5c44
dont use ritm singleton for iitm
JamieDanielson Mar 23, 2023
63477c5
fix up call for new iitm
JamieDanielson Mar 23, 2023
1188c02
Merge branch 'main' into esm-support
JamieDanielson Mar 23, 2023
33e544c
Merge branch 'main' into esm-support
lizthegrey Mar 28, 2023
bfe51f5
wrap function refactor
pkanal Mar 31, 2023
7110c37
clean up
pkanal Apr 3, 2023
3ddb3a9
Merge pull request #1 from honeycombio/purvi/esm-tests
JamieDanielson Apr 4, 2023
3f3d9a2
Merge branch 'main' into esm-support
JamieDanielson Apr 4, 2023
2936a16
update changelog: it takes a village
JamieDanielson Apr 4, 2023
4a10129
small cleanup on test
JamieDanielson Apr 4, 2023
50363c5
version should be node version, not instr version
JamieDanielson Apr 5, 2023
c9922c3
run lint:fix for prettier error
JamieDanielson Apr 5, 2023
b722486
Merge pull request #2 from honeycombio/jamie.http-node-version
pkanal Apr 5, 2023
4ac1ee1
base unwrap functionality
pkanal Apr 5, 2023
a346531
move esm specific wrap logic to node class
pkanal Apr 5, 2023
1ca6b55
fix types
pkanal Apr 6, 2023
4401576
add masswrap & massunwrap
pkanal Apr 6, 2023
86b6837
handle edge cases for masswrap and massunwrap
pkanal Apr 6, 2023
da694c7
Merge pull request #3 from honeycombio/purvi.unwrap
JamieDanielson Apr 7, 2023
1612855
Merge branch 'main' into esm-support
JamieDanielson Apr 7, 2023
e924c99
Ship shimmer types (#5)
pkanal Apr 25, 2023
b894bcd
Merge branch 'main' into esm-support
pkanal Apr 25, 2023
68c43c9
use --experimental-meta-resolve option with hook file (#8)
pkanal Apr 26, 2023
a2a74f8
Merge branch 'main' into esm-support
pkanal May 1, 2023
a529c8c
fix markdown
pkanal May 1, 2023
ddc8dc5
Merge branch 'main' into esm-support
pkanal May 3, 2023
42eb5ba
address PR feedback (#9)
pkanal May 3, 2023
4f71df8
Update experimental/packages/opentelemetry-instrumentation/hook.js
pkanal May 5, 2023
6a052b1
Update experimental/packages/opentelemetry-instrumentation/hook.mjs
pkanal May 5, 2023
58d34c0
change back to process.versions.node (#10)
pkanal May 5, 2023
33409da
address pr feedback (#11)
pkanal May 9, 2023
1f017b0
remove `--experimental-import-meta-resolve` flag (#12)
pkanal May 10, 2023
16109d3
Merge branch 'main' into esm-support
JamieDanielson May 10, 2023
bcaef1c
Merge branch 'main' into esm-support
JamieDanielson May 11, 2023
c590efc
Merge branch 'main' into esm-support
JamieDanielson May 12, 2023
59047bd
remove import limitation from README
pkanal May 12, 2023
0167a10
add esm example to examples readme
pkanal May 12, 2023
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
25 changes: 25 additions & 0 deletions examples/esm-http-ts/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Overview

This is a simple example that demonstrates tracing HTTP request, with an app written in TypeScript and transpiled to ES Modules.
Flarna marked this conversation as resolved.
Show resolved Hide resolved

## Installation

```sh
# from this directory
npm install
npm run build
npm start
```

In a separate terminal, `curl localhost:3000`.

See two spans in the console (one manual, one for http instrumentation)

## Useful links

- For more information on OpenTelemetry, visit: <https://opentelemetry.io/>
- For more information on OpenTelemetry for Node.js, visit: <https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-sdk-trace-node>

## LICENSE

Apache License 2.0
43 changes: 43 additions & 0 deletions examples/esm-http-ts/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { registerInstrumentations } from '@opentelemetry/instrumentation';
import { trace, DiagConsoleLogger, DiagLogLevel, diag } from '@opentelemetry/api';
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node';
import {
ConsoleSpanExporter,
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import { Resource } from '@opentelemetry/resources';
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
import http from 'http';
Flarna marked this conversation as resolved.
Show resolved Hide resolved

diag.setLogger(new DiagConsoleLogger(), DiagLogLevel.DEBUG);
const tracerProvider = new NodeTracerProvider({
resource: new Resource({
[SemanticResourceAttributes.SERVICE_NAME]: 'esm-http-ts-example',
}),
});
const exporter = new ConsoleSpanExporter();
const processor = new SimpleSpanProcessor(exporter);
tracerProvider.addSpanProcessor(processor);
tracerProvider.register();

registerInstrumentations({
instrumentations: [new HttpInstrumentation()],
});

const hostname = '0.0.0.0';
const port = 3000;

const server = http.createServer((req, res) => {
res.statusCode = 200;
res.setHeader('Content-Type', 'text/plain');
const tracer = trace.getTracer('esm-tracer');
tracer.startActiveSpan('manual', span => {
span.end();
});
res.end('Hello, World!\n');
});

server.listen(port, hostname, () => {
console.log(`Server running at http://${hostname}:${port}/`);
});
42 changes: 42 additions & 0 deletions examples/esm-http-ts/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
{
"name": "esm-http-ts",
"private": true,
"version": "0.38.0",
"description": "Example of HTTP integration with OpenTelemetry using ESM and TypeScript",
"main": "build/index.js",
"type": "module",
"scripts": {
"build": "tsc --build",
"start": "node --experimental-loader=@opentelemetry/instrumentation/hook.mjs ./build/index.js"
},
"repository": {
"type": "git",
"url": "git+ssh://git@github.com/open-telemetry/opentelemetry-js.git"
},
"keywords": [
"opentelemetry",
"http",
"tracing",
"esm",
"typescript"
],
"engines": {
"node": ">=14"
},
"author": "OpenTelemetry Authors",
"license": "Apache-2.0",
"bugs": {
"url": "https://github.com/open-telemetry/opentelemetry-js/issues"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/examples/",
"dependencies": {
"@opentelemetry/api": "1.4.0",
"@opentelemetry/exporter-trace-otlp-proto": "0.38.0",
"@opentelemetry/instrumentation": "0.38.0",
"@opentelemetry/instrumentation-http": "0.38.0",
"@opentelemetry/resources": "1.9.1",
"@opentelemetry/sdk-trace-base": "1.9.1",
"@opentelemetry/sdk-trace-node": "1.9.1",
"@opentelemetry/semantic-conventions": "1.9.1"
}
}
25 changes: 25 additions & 0 deletions examples/esm-http-ts/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"compilerOptions": {
/* Language and Environment */
"target": "ES2020" /* Set the JavaScript language version for emitted JavaScript and include compatible library declarations. */,
Flarna marked this conversation as resolved.
Show resolved Hide resolved

/* Modules */
"module": "ESNext" /* Specify what module code is generated. */,
"rootDir": "." /* Specify the root folder within your source files. */,
"moduleResolution": "node" /* Specify how TypeScript looks up a file from a given module specifier. */,
"resolveJsonModule": true /* Enable importing .json files. */,

/* Emit */
"outDir": "build" /* Specify an output folder for all emitted files. */,

/* Interop Constraints */
"allowSyntheticDefaultImports": true /* Allow 'import x from y' when a module doesn't have a default export. */,
"esModuleInterop": true /* Emit additional JavaScript to ease support for importing CommonJS modules. This enables 'allowSyntheticDefaultImports' for type compatibility. */,
"forceConsistentCasingInFileNames": true /* Ensure that casing is correct in imports. */,

/* Completeness */
"skipLibCheck": true /* Skip type checking all .d.ts files. */
Flarna marked this conversation as resolved.
Show resolved Hide resolved
},
"include": ["**/*.ts", "**/*.js", "*.config.js"],
"exclude": ["node_modules"]
}
2 changes: 2 additions & 0 deletions experimental/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ All notable changes to experimental packages in this project will be documented

### :rocket: (Enhancement)

* feat(instrumentation): add ESM support for instrumentation. [#3698](https://github.com/open-telemetry/opentelemetry-js/pull/3698) @JamieDanielson, @pkanal, @vmarchaud, @lizthegrey, @bengl

### :bug: (Bug Fix)

### :books: (Refine Doc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
export class HttpInstrumentation extends InstrumentationBase<Http> {
/** keep track on spans not ended */
private readonly _spanNotEnded: WeakSet<Span> = new WeakSet<Span>();
private readonly _version = process.versions.node;
private _headerCapture;
private _httpServerDurationHistogram!: Histogram;
private _httpClientDurationHistogram!: Histogram;
Expand Down Expand Up @@ -112,11 +111,12 @@ 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@${this._version}`);
this._diag.debug(`Applying patch for http@${version}`);
if (isWrapped(moduleExports.request)) {
this._unwrap(moduleExports, 'request');
}
Expand Down Expand Up @@ -145,7 +145,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
},
moduleExports => {
if (moduleExports === undefined) return;
this._diag.debug(`Removing patch for http@${this._version}`);
this._diag.debug(`Removing patch for http@${version}`);

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
Expand All @@ -155,11 +155,12 @@ 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@${this._version}`);
this._diag.debug(`Applying patch for https@${version}`);
if (isWrapped(moduleExports.request)) {
this._unwrap(moduleExports, 'request');
}
Expand Down Expand Up @@ -188,7 +189,7 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
},
moduleExports => {
if (moduleExports === undefined) return;
this._diag.debug(`Removing patch for https@${this._version}`);
this._diag.debug(`Removing patch for https@${version}`);

this._unwrap(moduleExports, 'request');
this._unwrap(moduleExports, 'get');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,13 @@ 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.

## Instrumentation for ES Modules In NodeJS (experimental)

As the module loading mechanism for ESM is different than CJS, you need to select a custom loader so instrumentation can load hook on the esm module it want to patch. To do so, you must provide the `--experimental-loader=@opentelemetry/instrumentation/hook.mjs` flag to the `node` binary. Alternatively you can set the `NODE_OPTIONS` environment variable to `NODE_OPTIONS="--experimental-loader=@opentelemetry/instrumentation/hook.mjs"`.
As the ESM module loader from NodeJS is experimental, so is our support for it. Feel free to provide feedback or report issues about it.
Flarna marked this conversation as resolved.
Show resolved Hide resolved

**Note**: ESM Instrumentation is not yet supported for Node 20.

Copy link
Member

Choose a reason for hiding this comment

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

Please update the limitations section below which tells ECMA script modules (using import) is not supported as of now.

Copy link

Choose a reason for hiding this comment

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

what does this mean? How can one use ESM without import?

Copy link
Member

Choose a reason for hiding this comment

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

It means that the limitations section requires changes in this PR because it removes one of them.
Before this PR ESM/import was not supported and only CJS/require worked.
Th limitation with bundlers stays.

Copy link

Choose a reason for hiding this comment

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

huh, super, thanks for the clarification :) a panic mode was already about to arise :D

## Limitations

Instrumentations for external modules (e.g. express, mongodb,...) hooks the `require` call. Therefore following conditions need to be met that this mechanism can work:
Expand Down
28 changes: 28 additions & 0 deletions experimental/packages/opentelemetry-instrumentation/hook.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*!
* Copyright 2021 Datadog, Inc.
* Copyright The OpenTelemetry Authors
pkanal marked this conversation as resolved.
Show resolved Hide resolved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2.0 License.
//
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021 Datadog, Inc.

import {
load,
resolve,
getFormat,
getSource,
} from 'import-in-the-middle/hook.mjs';
export { load, resolve, getFormat, getSource };
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"build/src/**/*.js",
"build/src/**/*.js.map",
"build/src/**/*.d.ts",
"hook.mjs",
"doc",
"LICENSE",
"README.md"
Expand All @@ -47,7 +48,9 @@
"tdd": "npm run tdd:node",
"tdd:node": "npm run test -- --watch-extensions ts --watch",
"tdd:browser": "karma start",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'",
"test:cjs": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts' --exclude 'test/browser/**/*.ts'",
"test:esm": "nyc node --experimental-loader=./hook.mjs ../../../node_modules/mocha/bin/mocha 'test/node/*.test.mjs' test/node/*.test.mjs",
"test": "npm run test:cjs && npm run test:esm",
"test:browser": "nyc karma start --single-run",
"version": "node ../../../scripts/version-update.js",
"watch": "tsc --build --watch tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
Expand All @@ -68,6 +71,8 @@
"url": "https://github.com/open-telemetry/opentelemetry-js/issues"
},
"dependencies": {
"@types/shimmer": "^1.0.2",
Flarna marked this conversation as resolved.
Show resolved Hide resolved
"import-in-the-middle": "1.3.5",
"require-in-the-middle": "^7.1.0",
"semver": "^7.3.2",
"shimmer": "^1.2.1"
Expand All @@ -82,7 +87,6 @@
"@types/mocha": "10.0.0",
"@types/node": "18.6.5",
"@types/semver": "7.3.9",
"@types/shimmer": "1.0.2",
"@types/sinon": "10.0.13",
"@types/webpack-env": "1.16.3",
"babel-loader": "8.2.3",
Expand Down
Loading