From 6bb45bf2e4c86187f095c8290f4fddbc6f01138d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Fri, 1 Nov 2024 13:54:15 -0400 Subject: [PATCH 1/8] Add Node.js support for --import flag --- apis/v1alpha1/instrumentation_types.go | 5 + autoinstrumentation/nodejs/package.json | 1 + .../nodejs/src/autoinstrumentation.mts | 4 + autoinstrumentation/nodejs/tsconfig.json | 2 +- ...emetry-operator.clusterserviceversion.yaml | 2 +- .../opentelemetry.io_instrumentations.yaml | 2 + ...emetry-operator.clusterserviceversion.yaml | 2 +- .../opentelemetry.io_instrumentations.yaml | 2 + .../opentelemetry.io_instrumentations.yaml | 2 + config/manager/kustomization.yaml | 12 ++ docs/api.md | 8 + pkg/instrumentation/nodejs.go | 11 +- pkg/instrumentation/nodejs_test.go | 114 +++++++++++ pkg/instrumentation/podmutator_test.go | 185 ++++++++++++++++++ .../00-install-collector.yaml | 22 +++ .../00-install-instrumentation.yaml | 34 ++++ .../01-assert.yaml | 77 ++++++++ .../01-install-app.yaml | 31 +++ .../chainsaw-test.yaml | 40 ++++ 19 files changed, 551 insertions(+), 5 deletions(-) create mode 100644 autoinstrumentation/nodejs/src/autoinstrumentation.mts create mode 100644 tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-collector.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-instrumentation.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-nodejs-import/01-assert.yaml create mode 100644 tests/e2e-instrumentation/instrumentation-nodejs-import/01-install-app.yaml create mode 100755 tests/e2e-instrumentation/instrumentation-nodejs-import/chainsaw-test.yaml diff --git a/apis/v1alpha1/instrumentation_types.go b/apis/v1alpha1/instrumentation_types.go index e290f4033b..7454a0b899 100644 --- a/apis/v1alpha1/instrumentation_types.go +++ b/apis/v1alpha1/instrumentation_types.go @@ -217,6 +217,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 that supports ESM. + // Requires Node.js 18 or later. + // +optional + UseImport bool `json:"useImport,omitempty"` } // Python defines Python SDK and instrumentation configuration. diff --git a/autoinstrumentation/nodejs/package.json b/autoinstrumentation/nodejs/package.json index a36adc8fad..8f02b48fd3 100644 --- a/autoinstrumentation/nodejs/package.json +++ b/autoinstrumentation/nodejs/package.json @@ -19,6 +19,7 @@ "@opentelemetry/exporter-metrics-otlp-grpc": "0.54.0", "@opentelemetry/exporter-prometheus": "0.54.0", "@opentelemetry/exporter-trace-otlp-grpc": "0.54.0", + "@opentelemetry/instrumentation": "0.54.0", "@opentelemetry/resource-detector-alibaba-cloud": "0.29.4", "@opentelemetry/resource-detector-aws": "1.7.0", "@opentelemetry/resource-detector-container": "0.5.0", diff --git a/autoinstrumentation/nodejs/src/autoinstrumentation.mts b/autoinstrumentation/nodejs/src/autoinstrumentation.mts new file mode 100644 index 0000000000..b9f9d4411d --- /dev/null +++ b/autoinstrumentation/nodejs/src/autoinstrumentation.mts @@ -0,0 +1,4 @@ +import "./autoinstrumentation.js"; + +import { register } from "node:module"; +register("@opentelemetry/instrumentation/hooks.mjs"); diff --git a/autoinstrumentation/nodejs/tsconfig.json b/autoinstrumentation/nodejs/tsconfig.json index 38012118db..b0090862d4 100644 --- a/autoinstrumentation/nodejs/tsconfig.json +++ b/autoinstrumentation/nodejs/tsconfig.json @@ -11,7 +11,7 @@ "forceConsistentCasingInFileNames": true, "incremental": true, "inlineSources": true, - "module": "commonjs", + "module": "node16", "newLine": "LF", "noEmitOnError": true, "noFallthroughCasesInSwitch": true, diff --git a/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml index b7007071dd..993c600be8 100644 --- a/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-10-30T17:23:26Z" + createdAt: "2024-11-01T17:13:20Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 diff --git a/bundle/community/manifests/opentelemetry.io_instrumentations.yaml b/bundle/community/manifests/opentelemetry.io_instrumentations.yaml index d8077d3867..988cda7a80 100644 --- a/bundle/community/manifests/opentelemetry.io_instrumentations.yaml +++ b/bundle/community/manifests/opentelemetry.io_instrumentations.yaml @@ -1496,6 +1496,8 @@ spec: x-kubernetes-int-or-string: true type: object type: object + useImport: + type: boolean volumeClaimTemplate: properties: metadata: diff --git a/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml index 5c19dabb99..9866177a69 100644 --- a/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-10-30T17:23:26Z" + createdAt: "2024-11-01T17:13:28Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 diff --git a/bundle/openshift/manifests/opentelemetry.io_instrumentations.yaml b/bundle/openshift/manifests/opentelemetry.io_instrumentations.yaml index d8077d3867..988cda7a80 100644 --- a/bundle/openshift/manifests/opentelemetry.io_instrumentations.yaml +++ b/bundle/openshift/manifests/opentelemetry.io_instrumentations.yaml @@ -1496,6 +1496,8 @@ spec: x-kubernetes-int-or-string: true type: object type: object + useImport: + type: boolean volumeClaimTemplate: properties: metadata: diff --git a/config/crd/bases/opentelemetry.io_instrumentations.yaml b/config/crd/bases/opentelemetry.io_instrumentations.yaml index 4032a33613..9353119680 100644 --- a/config/crd/bases/opentelemetry.io_instrumentations.yaml +++ b/config/crd/bases/opentelemetry.io_instrumentations.yaml @@ -1494,6 +1494,8 @@ spec: x-kubernetes-int-or-string: true type: object type: object + useImport: + type: boolean volumeClaimTemplate: properties: metadata: diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index 5c5f0b84cb..ede66a970a 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,2 +1,14 @@ resources: - manager.yaml +apiVersion: kustomize.config.k8s.io/v1beta1 +kind: Kustomization +images: +- name: controller + newName: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator +patches: +- patch: '[{"op":"add","path":"/spec/template/spec/containers/0/args/-","value":"--target-allocator-image=ghcr.io/open-telemetry/opentelemetry-operator/target-allocator:"}]' + target: + kind: Deployment +- patch: '[{"op":"add","path":"/spec/template/spec/containers/0/args/-","value":"--operator-opamp-bridge-image=ghcr.io/open-telemetry/opentelemetry-operator/operator-opamp-bridge:"}]' + target: + kind: Deployment diff --git a/docs/api.md b/docs/api.md index 9601cca2fd..114ba83df9 100644 --- a/docs/api.md +++ b/docs/api.md @@ -5666,6 +5666,14 @@ If the former var had been defined, then the other vars would be ignored.
Resources describes the compute resource requirements.
false + + useImport + boolean + + UseImport overrides the default injected --require flag with an --import flag that supports ESM. +Requires Node.js 18 or later.
+ + false volumeClaimTemplate object diff --git a/pkg/instrumentation/nodejs.go b/pkg/instrumentation/nodejs.go index a3d02ea53d..9be1dc6f60 100644 --- a/pkg/instrumentation/nodejs.go +++ b/pkg/instrumentation/nodejs.go @@ -23,6 +23,7 @@ import ( const ( envNodeOptions = "NODE_OPTIONS" nodeRequireArgument = " --require /otel-auto-instrumentation-nodejs/autoinstrumentation.js" + nodeImportArgument = " --import /otel-auto-instrumentation-nodejs/autoinstrumentation.mjs" nodejsInitContainerName = initContainerName + "-nodejs" nodejsVolumeName = volumeName + "-nodejs" nodejsInstrMountPath = "/otel-auto-instrumentation-nodejs" @@ -47,14 +48,20 @@ func injectNodeJSSDK(nodeJSSpec v1alpha1.NodeJS, pod corev1.Pod, index int) (cor } } + 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{ diff --git a/pkg/instrumentation/nodejs_test.go b/pkg/instrumentation/nodejs_test.go index 7ed4fcd6d3..bcc2b448dd 100644 --- a/pkg/instrumentation/nodejs_test.go +++ b/pkg/instrumentation/nodejs_test.go @@ -179,6 +179,120 @@ func TestInjectNodeJSSDK(t *testing.T) { }, err: fmt.Errorf("the container defines env var value via ValueFrom, envVar: %s", envNodeOptions), }, + { + 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.mjs", + }, + }, + }, + }, + }, + }, + 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.mjs", + }, + }, + }, + }, + }, + }, + err: nil, + }, } for _, test := range tests { diff --git a/pkg/instrumentation/podmutator_test.go b/pkg/instrumentation/podmutator_test.go index 3fd085d539..94d8e0d0f5 100644 --- a/pkg/instrumentation/podmutator_test.go +++ b/pkg/instrumentation/podmutator_test.go @@ -876,6 +876,191 @@ func TestMutatePod(t *testing.T) { }, config: config.New(config.WithEnableNodeJSInstrumentation(true)), }, + { + name: "nodejs injection use import, true", + ns: corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nodejs-import", + }, + }, + inst: v1alpha1.Instrumentation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "example-inst", + Namespace: "nodejs-import", + }, + Spec: v1alpha1.InstrumentationSpec{ + NodeJS: v1alpha1.NodeJS{ + Image: "otel/nodejs:1", + Env: []corev1.EnvVar{ + { + Name: "OTEL_NODEJS_DEBUG", + Value: "true", + }, + }, + UseImport: true, + }, + Env: []corev1.EnvVar{ + { + Name: "OTEL_TRACES_EXPORTER", + Value: "otlp", + }, + { + Name: "OTEL_EXPORTER_OTLP_ENDPOINT", + Value: "http://localhost:4317", + }, + { + Name: "OTEL_EXPORTER_OTLP_TIMEOUT", + Value: "20", + }, + { + Name: "OTEL_TRACES_SAMPLER", + Value: "parentbased_traceidratio", + }, + { + Name: "OTEL_TRACES_SAMPLER_ARG", + Value: "0.85", + }, + { + Name: "SPLUNK_TRACE_RESPONSE_HEADER_ENABLED", + Value: "true", + }, + }, + Exporter: v1alpha1.Exporter{ + Endpoint: "http://collector:12345", + }, + }, + }, + pod: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationInjectNodeJS: "true", + }, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "app", + }, + }, + }, + }, + expected: corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + annotationInjectNodeJS: "true", + }, + }, + Spec: corev1.PodSpec{ + Volumes: []corev1.Volume{ + { + Name: nodejsVolumeName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{ + SizeLimit: &defaultVolumeLimitSize, + }, + }, + }, + }, + InitContainers: []corev1.Container{ + { + Name: nodejsInitContainerName, + Image: "otel/nodejs:1", + Command: []string{"cp", "-r", "/autoinstrumentation/.", nodejsInstrMountPath}, + VolumeMounts: []corev1.VolumeMount{{ + Name: nodejsVolumeName, + MountPath: nodejsInstrMountPath, + }}, + }, + }, + Containers: []corev1.Container{ + { + Name: "app", + Env: []corev1.EnvVar{ + { + Name: "OTEL_NODE_IP", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "status.hostIP", + }, + }, + }, + { + Name: "OTEL_POD_IP", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "status.podIP", + }, + }, + }, + { + Name: "OTEL_NODEJS_DEBUG", + Value: "true", + }, + { + Name: "NODE_OPTIONS", + Value: nodeImportArgument, + }, + { + Name: "OTEL_TRACES_EXPORTER", + Value: "otlp", + }, + { + Name: "OTEL_EXPORTER_OTLP_ENDPOINT", + Value: "http://localhost:4317", + }, + { + Name: "OTEL_EXPORTER_OTLP_TIMEOUT", + Value: "20", + }, + { + Name: "OTEL_TRACES_SAMPLER", + Value: "parentbased_traceidratio", + }, + { + Name: "OTEL_TRACES_SAMPLER_ARG", + Value: "0.85", + }, + { + Name: "SPLUNK_TRACE_RESPONSE_HEADER_ENABLED", + Value: "true", + }, + { + Name: "OTEL_SERVICE_NAME", + Value: "app", + }, + { + Name: "OTEL_RESOURCE_ATTRIBUTES_POD_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "metadata.name", + }, + }, + }, + { + Name: "OTEL_RESOURCE_ATTRIBUTES_NODE_NAME", + ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{ + FieldPath: "spec.nodeName", + }, + }, + }, + { + Name: "OTEL_RESOURCE_ATTRIBUTES", + Value: "k8s.container.name=app,k8s.namespace.name=nodejs-import,k8s.node.name=$(OTEL_RESOURCE_ATTRIBUTES_NODE_NAME),k8s.pod.name=$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME),service.instance.id=nodejs-import.$(OTEL_RESOURCE_ATTRIBUTES_POD_NAME).app", + }, + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: nodejsVolumeName, + MountPath: nodejsInstrMountPath, + }, + }, + }, + }, + }, + }, + config: config.New(config.WithEnableNodeJSInstrumentation(true)), + }, { name: "nodejs injection multiple containers, true", ns: corev1.Namespace{ diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-collector.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-collector.yaml new file mode 100644 index 0000000000..34a26ebb2c --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-collector.yaml @@ -0,0 +1,22 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: OpenTelemetryCollector +metadata: + name: sidecar +spec: + config: | + receivers: + otlp: + protocols: + grpc: + http: + processors: + + exporters: + debug: + + service: + pipelines: + traces: + receivers: [otlp] + exporters: [debug] + mode: sidecar diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-instrumentation.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-instrumentation.yaml new file mode 100644 index 0000000000..e86b9a6f52 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-nodejs-import/00-install-instrumentation.yaml @@ -0,0 +1,34 @@ +apiVersion: opentelemetry.io/v1alpha1 +kind: Instrumentation +metadata: + name: nodejs-import +spec: + env: + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + - name: OTEL_METRICS_EXPORTER + value: prometheus + exporter: + endpoint: http://localhost:4317 + propagators: + - jaeger + - b3 + sampler: + type: parentbased_traceidratio + argument: "0.25" + nodejs: + useImport: true + env: + - name: OTEL_NODEJS_DEBUG + value: "true" + diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-import/01-assert.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-import/01-assert.yaml new file mode 100644 index 0000000000..f5147d5456 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-nodejs-import/01-assert.yaml @@ -0,0 +1,77 @@ +apiVersion: v1 +kind: Pod +metadata: + annotations: + instrumentation.opentelemetry.io/inject-nodejs: "true" + sidecar.opentelemetry.io/inject: "true" + labels: + app: my-nodejs +spec: + containers: + - env: + - name: OTEL_NODE_IP + valueFrom: + fieldRef: + fieldPath: status.hostIP + - name: OTEL_POD_IP + valueFrom: + fieldRef: + fieldPath: status.podIP + - name: NODE_PATH + value: /usr/local/lib/node_modules + - name: OTEL_NODEJS_DEBUG + value: "true" + - name: NODE_OPTIONS + value: ' --import /otel-auto-instrumentation-nodejs/autoinstrumentation.mjs' + - name: OTEL_TRACES_EXPORTER + value: otlp + - name: OTEL_EXPORTER_OTLP_ENDPOINT + value: http://localhost:4317 + - name: OTEL_EXPORTER_OTLP_TIMEOUT + value: "20" + - name: OTEL_TRACES_SAMPLER + value: parentbased_traceidratio + - name: OTEL_TRACES_SAMPLER_ARG + value: "0.85" + - name: SPLUNK_TRACE_RESPONSE_HEADER_ENABLED + value: "true" + - name: OTEL_METRICS_EXPORTER + value: prometheus + - name: OTEL_SERVICE_NAME + value: my-nodejs + - name: OTEL_RESOURCE_ATTRIBUTES_POD_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: metadata.name + - name: OTEL_RESOURCE_ATTRIBUTES_NODE_NAME + valueFrom: + fieldRef: + apiVersion: v1 + fieldPath: spec.nodeName + - name: OTEL_PROPAGATORS + value: jaeger,b3 + - name: OTEL_RESOURCE_ATTRIBUTES + name: myapp + volumeMounts: + - mountPath: /var/run/secrets/kubernetes.io/serviceaccount + readOnly: true + - mountPath: /otel-auto-instrumentation-nodejs + name: opentelemetry-auto-instrumentation-nodejs + - args: + - --config=env:OTEL_CONFIG + name: otc-container + initContainers: + - name: opentelemetry-auto-instrumentation-nodejs +status: + containerStatuses: + - name: myapp + ready: true + started: true + - name: otc-container + ready: true + started: true + initContainerStatuses: + - name: opentelemetry-auto-instrumentation-nodejs + ready: true + phase: Running diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-import/01-install-app.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-import/01-install-app.yaml new file mode 100644 index 0000000000..45de0d5f01 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-nodejs-import/01-install-app.yaml @@ -0,0 +1,31 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: my-nodejs +spec: + selector: + matchLabels: + app: my-nodejs + replicas: 1 + template: + metadata: + labels: + app: my-nodejs + annotations: + sidecar.opentelemetry.io/inject: "true" + instrumentation.opentelemetry.io/inject-nodejs: "true" + spec: + securityContext: + runAsUser: 1000 + runAsGroup: 3000 + fsGroup: 3000 + containers: + - name: myapp + image: ghcr.io/open-telemetry/opentelemetry-operator/e2e-test-app-nodejs:main + securityContext: + allowPrivilegeEscalation: false + capabilities: + drop: ["ALL"] + env: + - name: NODE_PATH + value: /usr/local/lib/node_modules diff --git a/tests/e2e-instrumentation/instrumentation-nodejs-import/chainsaw-test.yaml b/tests/e2e-instrumentation/instrumentation-nodejs-import/chainsaw-test.yaml new file mode 100755 index 0000000000..3158e5d007 --- /dev/null +++ b/tests/e2e-instrumentation/instrumentation-nodejs-import/chainsaw-test.yaml @@ -0,0 +1,40 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: instrumentation-nodejs-import +spec: + steps: + - name: step-00 + try: + # In OpenShift, when a namespace is created, all necessary SCC annotations are automatically added. However, if a namespace is created using a resource file with only selected SCCs, the other auto-added SCCs are not included. Therefore, the UID-range and supplemental groups SCC annotations must be set after the namespace is created. + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.uid-range=1000/1000 + - --overwrite + - command: + entrypoint: kubectl + args: + - annotate + - namespace + - ${NAMESPACE} + - openshift.io/sa.scc.supplemental-groups=3000/3000 + - --overwrite + - apply: + file: 00-install-collector.yaml + - apply: + file: 00-install-instrumentation.yaml + - name: step-01 + try: + - apply: + file: 01-install-app.yaml + - assert: + file: 01-assert.yaml + catch: + - podLogs: + selector: app=my-nodejs From 30c7f36d37ab83a1de1763daf1f5375d7d113885 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Fri, 1 Nov 2024 14:26:41 -0400 Subject: [PATCH 2/8] Fix tsconfig for Node.js autoinstrumentation --- .chloggen/node-import.yaml | 16 ++++++++++++++++ autoinstrumentation/nodejs/tsconfig.json | 1 + ...telemetry-operator.clusterserviceversion.yaml | 4 +++- ...telemetry-operator.clusterserviceversion.yaml | 2 +- 4 files changed, 21 insertions(+), 2 deletions(-) create mode 100755 .chloggen/node-import.yaml diff --git a/.chloggen/node-import.yaml b/.chloggen/node-import.yaml new file mode 100755 index 0000000000..cadc4bbb8e --- /dev/null +++ b/.chloggen/node-import.yaml @@ -0,0 +1,16 @@ +# 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: diff --git a/autoinstrumentation/nodejs/tsconfig.json b/autoinstrumentation/nodejs/tsconfig.json index b0090862d4..79c4a12f93 100644 --- a/autoinstrumentation/nodejs/tsconfig.json +++ b/autoinstrumentation/nodejs/tsconfig.json @@ -27,6 +27,7 @@ }, "include": [ "src/**/*.ts", + "src/**/*.mts" ], "exclude": [ "node_modules" diff --git a/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml index 993c600be8..a0a161dcde 100644 --- a/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-11-01T17:13:20Z" + createdAt: "2024-11-01T18:25:51Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 @@ -478,6 +478,8 @@ spec: - --zap-log-level=info - --zap-time-encoding=rfc3339nano - --enable-nginx-instrumentation=true + - '--target-allocator-image=ghcr.io/open-telemetry/opentelemetry-operator/target-allocator:' + - '--operator-opamp-bridge-image=ghcr.io/open-telemetry/opentelemetry-operator/operator-opamp-bridge:' env: - name: SERVICE_ACCOUNT_NAME valueFrom: diff --git a/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml index 9866177a69..15d1ed536f 100644 --- a/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-11-01T17:13:28Z" + createdAt: "2024-11-01T18:25:59Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 From f4e5a79870c8b7d1428109542c8bdef76780359e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Mon, 4 Nov 2024 13:43:53 -0500 Subject: [PATCH 3/8] add subtext to changelog --- .chloggen/node-import.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.chloggen/node-import.yaml b/.chloggen/node-import.yaml index cadc4bbb8e..1a60ff01e2 100755 --- a/.chloggen/node-import.yaml +++ b/.chloggen/node-import.yaml @@ -13,4 +13,6 @@ 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: +subtext: | + The new UseImport configuration overrides the default injected --require flag with an --import flag that supports ESM. + Requires Node.js 18 or later. From 3b65d654b9cfcb570bf78d9e7ff05224f322bfca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Tue, 5 Nov 2024 13:03:06 -0500 Subject: [PATCH 4/8] fix kustomization --- config/manager/kustomization.yaml | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/config/manager/kustomization.yaml b/config/manager/kustomization.yaml index ede66a970a..5c5f0b84cb 100644 --- a/config/manager/kustomization.yaml +++ b/config/manager/kustomization.yaml @@ -1,14 +1,2 @@ resources: - manager.yaml -apiVersion: kustomize.config.k8s.io/v1beta1 -kind: Kustomization -images: -- name: controller - newName: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator -patches: -- patch: '[{"op":"add","path":"/spec/template/spec/containers/0/args/-","value":"--target-allocator-image=ghcr.io/open-telemetry/opentelemetry-operator/target-allocator:"}]' - target: - kind: Deployment -- patch: '[{"op":"add","path":"/spec/template/spec/containers/0/args/-","value":"--operator-opamp-bridge-image=ghcr.io/open-telemetry/opentelemetry-operator/operator-opamp-bridge:"}]' - target: - kind: Deployment From 993d67f21e162e2310eeb67be9874ab14d7b165d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Wed, 6 Nov 2024 12:02:59 -0500 Subject: [PATCH 5/8] run reset again --- .../opentelemetry-operator.clusterserviceversion.yaml | 4 +--- .../opentelemetry-operator.clusterserviceversion.yaml | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml index a0a161dcde..096b87b595 100644 --- a/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/community/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-11-01T18:25:51Z" + createdAt: "2024-11-06T16:57:03Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 @@ -478,8 +478,6 @@ spec: - --zap-log-level=info - --zap-time-encoding=rfc3339nano - --enable-nginx-instrumentation=true - - '--target-allocator-image=ghcr.io/open-telemetry/opentelemetry-operator/target-allocator:' - - '--operator-opamp-bridge-image=ghcr.io/open-telemetry/opentelemetry-operator/operator-opamp-bridge:' env: - name: SERVICE_ACCOUNT_NAME valueFrom: diff --git a/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml b/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml index 15d1ed536f..fd6e396024 100644 --- a/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml +++ b/bundle/openshift/manifests/opentelemetry-operator.clusterserviceversion.yaml @@ -99,7 +99,7 @@ metadata: categories: Logging & Tracing,Monitoring certified: "false" containerImage: ghcr.io/open-telemetry/opentelemetry-operator/opentelemetry-operator - createdAt: "2024-11-01T18:25:59Z" + createdAt: "2024-11-06T16:57:03Z" description: Provides the OpenTelemetry components, including the Collector operators.operatorframework.io/builder: operator-sdk-v1.29.0 operators.operatorframework.io/project_layout: go.kubebuilder.io/v3 From 555ddcb8e8bc8afbb38c4142b49dc49828b258ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Fri, 8 Nov 2024 15:21:03 -0500 Subject: [PATCH 6/8] address JS import flag review --- .chloggen/node-import.yaml | 6 ++++-- apis/v1alpha1/instrumentation_types.go | 6 ++++-- autoinstrumentation/nodejs/Dockerfile | 2 ++ autoinstrumentation/nodejs/src/autoinstrumentation.mts | 10 ++++++++-- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/.chloggen/node-import.yaml b/.chloggen/node-import.yaml index 1a60ff01e2..3e244f0649 100755 --- a/.chloggen/node-import.yaml +++ b/.chloggen/node-import.yaml @@ -14,5 +14,7 @@ issues: [3414] # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. subtext: | - The new UseImport configuration overrides the default injected --require flag with an --import flag that supports ESM. - Requires Node.js 18 or later. + The new UseImport option overrides the default injected --require flag with an --import flag. + When using the provided container image, this enables instrumentation of ESM code. + This behavior may be different for other images, and the option may even be either required or unsupported. + Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported. diff --git a/apis/v1alpha1/instrumentation_types.go b/apis/v1alpha1/instrumentation_types.go index 7454a0b899..72472a3b95 100644 --- a/apis/v1alpha1/instrumentation_types.go +++ b/apis/v1alpha1/instrumentation_types.go @@ -218,8 +218,10 @@ type NodeJS struct { // +optional Resources corev1.ResourceRequirements `json:"resourceRequirements,omitempty"` - // UseImport overrides the default injected --require flag with an --import flag that supports ESM. - // Requires Node.js 18 or later. + // UseImport overrides the default injected --require flag with an --import flag. + // When using the provided container image, this enables instrumentation of ESM code. + // This behavior may be different for other images, and the option may even be either required or unsupported. + // Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported. // +optional UseImport bool `json:"useImport,omitempty"` } diff --git a/autoinstrumentation/nodejs/Dockerfile b/autoinstrumentation/nodejs/Dockerfile index 48f1f9ae75..b496178603 100644 --- a/autoinstrumentation/nodejs/Dockerfile +++ b/autoinstrumentation/nodejs/Dockerfile @@ -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 availabe 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 a file name autoinstrumentation.mjs using `import`. FROM node:20 AS build WORKDIR /operator-build diff --git a/autoinstrumentation/nodejs/src/autoinstrumentation.mts b/autoinstrumentation/nodejs/src/autoinstrumentation.mts index b9f9d4411d..dc28465aa2 100644 --- a/autoinstrumentation/nodejs/src/autoinstrumentation.mts +++ b/autoinstrumentation/nodejs/src/autoinstrumentation.mts @@ -1,4 +1,10 @@ import "./autoinstrumentation.js"; +import module from "node:module"; -import { register } from "node:module"; -register("@opentelemetry/instrumentation/hooks.mjs"); +if (typeof module.register === "function") { + module.register("@opentelemetry/instrumentation/hooks.mjs"); +} else { + console.warn( + `OpenTelemetry Operator auto-instrumentation could not instrument ESM code: Node.js ${process.version} does not support 'module.register()'` + ); +} From a8e0a47fb094d3a58da7f8b6a51b204770990bff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Fri, 8 Nov 2024 15:22:59 -0500 Subject: [PATCH 7/8] update docs --- docs/api.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/api.md b/docs/api.md index 114ba83df9..574e33d218 100644 --- a/docs/api.md +++ b/docs/api.md @@ -5670,8 +5670,10 @@ If the former var had been defined, then the other vars would be ignored.
useImport boolean - UseImport overrides the default injected --require flag with an --import flag that supports ESM. -Requires Node.js 18 or later.
+ UseImport overrides the default injected --require flag with an --import flag. +When using the provided container image, this enables instrumentation of ESM code. +This behavior may be different for other images, and the option may even be either required or unsupported. +Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported.
false From da2827e9c2e97080d7cbbb77e55519b115feeacb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?= Date: Thu, 21 Nov 2024 12:04:36 -0500 Subject: [PATCH 8/8] use spaces in changelog --- .chloggen/node-import.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.chloggen/node-import.yaml b/.chloggen/node-import.yaml index 3e244f0649..3fff3a2831 100755 --- a/.chloggen/node-import.yaml +++ b/.chloggen/node-import.yaml @@ -15,6 +15,6 @@ issues: [3414] # Use pipe (|) for multiline entries. subtext: | The new UseImport option overrides the default injected --require flag with an --import flag. - When using the provided container image, this enables instrumentation of ESM code. - This behavior may be different for other images, and the option may even be either required or unsupported. - Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported. + When using the provided container image, this enables instrumentation of ESM code. + This behavior may be different for other images, and the option may even be either required or unsupported. + Node.js ^18.19.0 || ^20.6.0 || >=22 is required for the flag to be supported.