Skip to content

Add Node.js support for --import flag #3416

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

Open
raphael-theriault-swi wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add Node.js support for --import flag #3416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc3a331
Add Node.js support for --import flag
raphael-theriault-swi Jul 8, 2025
1fea4ce
Merge branch 'main' into node-import
swiatekm Oct 28, 2025
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
18 changes: 18 additions & 0 deletions .chloggen/node-import.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
component: auto-instrumentation

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add Node.js support for `--import` flag

# One or more tracking issues related to the change
issues: [3414]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
The new UseImport option overrides the default injected --require flag with an --import flag.
Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported.
5 changes: 5 additions & 0 deletions apis/v1alpha1/instrumentation_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,11 @@ type NodeJS struct {
// Resources describes the compute resource requirements.
// +optional
Resources corev1.ResourceRequirements `json:"resourceRequirements,omitempty"`

// UseImport overrides the default injected --require flag with an --import flag.
// Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported.
// +optional
UseImport bool `json:"useImport,omitempty"`
Copy link
Copy Markdown

@trentm trentm Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

One subtlety that might impact the choice of the config var name.
The feature for the user is whether a module import hook is added (via module.register()) to support instrumentating ESM code.

That is actually independent of whether --import something.mjs or --require something.js is used to run the OTel setup when Node.js is starting up. It is definitely possible to call module.register("@opentelemetry/instrumentation/hooks.mjs"); from the CommonJS autoinstrumentation.js (loaded via --require).

So I wonder if a config name something like instrumentEsm or something like that would be clearer.

There are a few subtleties discussed at open-telemetry/opentelemetry-js#4933, but I don't think you need to read and grok all that. Eventually we'd like it so that there is just the one obvious way to setup OTel -- and that would be via --import some-esm-file-that-calls-module.register.mjs. However, for now we need to make the ESM support opt-in because:

  • it requires newer versions of node
  • it is possible the user explicitly does not want ESM instrumentation enabled because it is still new and can have overhead and bugs

Copy link
Copy Markdown
Member Author

@raphael-theriault-swi raphael-theriault-swi Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think instrumentEsm here would be a bit of a misnomer, given that functionality could be added without an import flag, and a custom image could also require the use of an import flag for top level await support (which is actually our usecase for this addition) without providing ESM instrumentation

Copy link
Copy Markdown

@trentm trentm Nov 8, 2024

Choose a reason for hiding this comment

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

a custom image could also require the use of an import flag for top level await support (which is actually our usecase for this addition)

Ah, I misunderstood the use case then. Using import in the var name makes sense then. The docs for this feature should mention the two things then: (1) --import ... is used to load "autoinstrumentation.mjs" (so custom versions can use ESM and top-level await), (2) the default "autoinstrumentation.mjs" differs from "autoinstrumentation.js" in that it registers the hook for ESM instrumentation.

}

// Python defines Python SDK and instrumentation configuration.
Expand Down
2 changes: 2 additions & 0 deletions autoinstrumentation/nodejs/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
# - Grant the necessary access to `/autoinstrumentation` directory. `chmod -R go+r /autoinstrumentation`
# - For auto-instrumentation by container injection, the Linux command cp is
# used and must be available in the image.
# - By default a file named autoinstrumentation.js is loaded using `require`, but the UseImport configuration option
# overrides this default behaviour and loads it autoinstrumentation.mjs using `import`.
FROM node:20 AS build

WORKDIR /operator-build
Expand Down
5 changes: 2 additions & 3 deletions autoinstrumentation/nodejs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@
"private": true,
"scripts": {
"clean": "rimraf build/*",
"postinstall": "copyfiles -f 'build/src/**' build/workspace/ && copyfiles 'node_modules/**' package.json build/workspace/ && npm -C build/workspace prune --omit=dev --no-package-lock"
"postinstall": "copyfiles 'node_modules/**' package.json build/workspace/ && npm -C build/workspace prune --omit=dev --no-package-lock"
},
"devDependencies": {
"copyfiles": "^2.4.1",
"rimraf": "^6.0.1",
"typescript": "^5.6.3"
"rimraf": "^6.0.1"
},
"dependencies": {
"@opentelemetry/api": "1.9.0",
Expand Down
34 changes: 0 additions & 34 deletions autoinstrumentation/nodejs/tsconfig.json
View file
Open in desktop

This file was deleted.

2 changes: 2 additions & 0 deletions bundle/community/manifests/opentelemetry.io_instrumentations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,6 +1498,8 @@ spec:
x-kubernetes-int-or-string: true
type: object
type: object
useImport:
type: boolean
volumeClaimTemplate:
properties:
metadata:
Expand Down
2 changes: 2 additions & 0 deletions bundle/openshift/manifests/opentelemetry.io_instrumentations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1498,6 +1498,8 @@ spec:
x-kubernetes-int-or-string: true
type: object
type: object
useImport:
type: boolean
volumeClaimTemplate:
properties:
metadata:
Expand Down
2 changes: 2 additions & 0 deletions config/crd/bases/opentelemetry.io_instrumentations.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,8 @@ spec:
x-kubernetes-int-or-string: true
type: object
type: object
useImport:
type: boolean
volumeClaimTemplate:
properties:
metadata:
Expand Down
8 changes: 8 additions & 0 deletions docs/api/instrumentations.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5675,6 +5675,14 @@ If the former var had been defined, then the other vars would be ignored.<br/>
Resources describes the compute resource requirements.<br/>
</td>
<td>false</td>
</tr><tr>
<td><b>useImport</b></td>
<td>boolean</td>
<td>
UseImport overrides the default injected --require flag with an --import flag.
Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported.<br/>
</td>
<td>false</td>
</tr><tr>
<td><b><a href="#instrumentationspecnodejsvolumeclaimtemplate">volumeClaimTemplate</a></b></td>
<td>object</td>
Expand Down
11 changes: 9 additions & 2 deletions internal/instrumentation/nodejs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
const (
envNodeOptions = "NODE_OPTIONS"
nodeRequireArgument = " --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js"
nodeImportArgument = " --import /otel-auto-instrumentation-nodejs/autoinstrumentation.js"
nodejsInitContainerName = initContainerName + "-nodejs"
nodejsVolumeName = volumeName + "-nodejs"
nodejsInstrMountPath = "/otel-auto-instrumentation-nodejs"
Expand All @@ -31,14 +32,20 @@ func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, index int, inst
// inject NodeJS instrumentation spec env vars.
container.Env = appendIfNotSet(container.Env, nodeJSSpec.Env...)

var nodeArgument string
if nodeJSSpec.UseImport {
nodeArgument = nodeImportArgument
} else {
nodeArgument = nodeRequireArgument
}
idx := getIndexOfEnv(container.Env, envNodeOptions)
if idx == -1 {
container.Env = append(container.Env, corev1.EnvVar{
Name: envNodeOptions,
Value: nodeRequireArgument,
Value: nodeArgument,
})
} else if idx > -1 {
container.Env[idx].Value = container.Env[idx].Value + nodeRequireArgument
container.Env[idx].Value = container.Env[idx].Value + nodeArgument
}

container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{
Expand Down
114 changes: 114 additions & 0 deletions internal/instrumentation/nodejs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,120 @@ func TestInjectNodeJSSDK(t *testing.T) {
},
err: nil,
},
{
name: "NODE_OPTIONS not defined and UseImport true",
NodeJS: v1alpha1.NodeJS{Image: "foo/bar:1", UseImport: true},
pod: corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{},
},
},
},
expected: corev1.Pod{
Spec: corev1.PodSpec{
Volumes: []corev1.Volume{
{
Name: "opentelemetry-auto-instrumentation-nodejs",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{
SizeLimit: &defaultVolumeLimitSize,
},
},
},
},
InitContainers: []corev1.Container{
{
Name: "opentelemetry-auto-instrumentation-nodejs",
Image: "foo/bar:1",
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-nodejs"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-nodejs",
MountPath: "/otel-auto-instrumentation-nodejs",
}},
},
},
Containers: []corev1.Container{
{
VolumeMounts: []corev1.VolumeMount{
{
Name: "opentelemetry-auto-instrumentation-nodejs",
MountPath: "/otel-auto-instrumentation-nodejs",
},
},
Env: []corev1.EnvVar{
{
Name: "NODE_OPTIONS",
Value: " --import /otel-auto-instrumentation-nodejs/autoinstrumentation.js",
},
},
},
},
},
},
err: nil,
},
{
name: "NODE_OPTIONS defined and UseImport true",
NodeJS: v1alpha1.NodeJS{Image: "foo/bar:1", Resources: testResourceRequirements, UseImport: true},
pod: corev1.Pod{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Env: []corev1.EnvVar{
{
Name: "NODE_OPTIONS",
Value: "-Dbaz=bar",
},
},
},
},
},
},
expected: corev1.Pod{
Spec: corev1.PodSpec{
Volumes: []corev1.Volume{
{
Name: "opentelemetry-auto-instrumentation-nodejs",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{
SizeLimit: &defaultVolumeLimitSize,
},
},
},
},
InitContainers: []corev1.Container{
{
Name: "opentelemetry-auto-instrumentation-nodejs",
Image: "foo/bar:1",
Command: []string{"cp", "-r", "/autoinstrumentation/.", "/otel-auto-instrumentation-nodejs"},
VolumeMounts: []corev1.VolumeMount{{
Name: "opentelemetry-auto-instrumentation-nodejs",
MountPath: "/otel-auto-instrumentation-nodejs",
}},
Resources: testResourceRequirements,
},
},
Containers: []corev1.Container{
{
VolumeMounts: []corev1.VolumeMount{
{
Name: "opentelemetry-auto-instrumentation-nodejs",
MountPath: "/otel-auto-instrumentation-nodejs",
},
},
Env: []corev1.EnvVar{
{
Name: "NODE_OPTIONS",
Value: "-Dbaz=bar" + " --import /otel-auto-instrumentation-nodejs/autoinstrumentation.js",
},
},
},
},
},
},
err: nil,
},
{
name: "NODE_OPTIONS defined as ValueFrom",
NodeJS: v1alpha1.NodeJS{Image: "foo/bar:1"},
Expand Down
Loading
Loading