From 9fa42c7f1534697384e1189ba68a7df7f90744bb Mon Sep 17 00:00:00 2001 From: Marc Pichler Date: Wed, 3 Jan 2024 17:41:23 +0100 Subject: [PATCH] fix(instrumentation): do not import 'path' in browser runtimes (#4386) * fix(instrumentation): do not import 'path' in browser runtimes * fix(changelog): clean up and add entry * fix(instrumentation): add missing license header * fix(changelog): formatting * fix(instrumentation): do not throw --- experimental/CHANGELOG.md | 4 ++- .../src/instrumentationNodeModuleFile.ts | 2 +- .../src/platform/browser/index.ts | 1 + .../src/platform/browser/noop-normalize.ts | 35 +++++++++++++++++++ .../src/platform/index.ts | 2 +- .../src/platform/node/index.ts | 1 + .../src/platform/node/normalize.ts | 17 +++++++++ .../test/browser/noop-normalize.test.ts | 24 +++++++++++++ 8 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 experimental/packages/opentelemetry-instrumentation/src/platform/browser/noop-normalize.ts create mode 100644 experimental/packages/opentelemetry-instrumentation/src/platform/node/normalize.ts create mode 100644 experimental/packages/opentelemetry-instrumentation/test/browser/noop-normalize.test.ts diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 4ffc9cdc6f..abec890a38 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -10,7 +10,9 @@ All notable changes to experimental packages in this project will be documented ### :bug: (Bug Fix) -fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc +* fix(instrumentation): use caret range on import-in-the-middle [#4380](https://github.com/open-telemetry/opentelemetry-js/pull/4380) @pichlermarc +* fix(instrumentation): do not import 'path' in browser runtimes [#4378](https://github.com/open-telemetry/opentelemetry-js/pull/4378) @pichlermarc + * Fixes a bug where bundling for web would fail due to `InstrumentationNodeModuleDefinition` importing `path` ### :books: (Refine Doc) diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleFile.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleFile.ts index 574f17257e..edbe8ba72e 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleFile.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentationNodeModuleFile.ts @@ -15,7 +15,7 @@ */ import { InstrumentationModuleFile } from './types'; -import { normalize } from 'path'; +import { normalize } from './platform/index'; export class InstrumentationNodeModuleFile implements InstrumentationModuleFile diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/index.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/index.ts index 0b238b42b8..4ad5c8f063 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/index.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/index.ts @@ -15,3 +15,4 @@ */ export { InstrumentationBase } from './instrumentation'; +export { normalize } from './noop-normalize'; diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/noop-normalize.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/noop-normalize.ts new file mode 100644 index 0000000000..7b8aa40000 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/noop-normalize.ts @@ -0,0 +1,35 @@ +/* + * Copyright The OpenTelemetry Authors + * + * 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 + * + * https://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. + */ + +import { diag } from '@opentelemetry/api'; + +/** + * Placeholder normalize function to replace the node variant in browser runtimes, + * this should never be called and will perform a no-op and warn if it is called regardless. + * + * This is a workaround to fix https://github.com/open-telemetry/opentelemetry-js/issues/4373 until the instrumentation + * package can be made node-only. + * + * @param path input path + * @return unmodified path + * @internal + */ +export function normalize(path: string): string { + diag.warn( + 'Path normalization is not implemented for this platform. To silence this warning, ensure no node-specific instrumentations are loaded, and node-specific types (e.g. InstrumentationNodeModuleFile), are not used in a browser context)' + ); + return path; +} diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/index.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/index.ts index 81d3096252..f24b70eac5 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/index.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export { InstrumentationBase } from './node'; +export { InstrumentationBase, normalize } from './node'; diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/index.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/index.ts index 1e81931b2a..94f517dfa3 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/index.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/index.ts @@ -14,3 +14,4 @@ * limitations under the License. */ export { InstrumentationBase } from './instrumentation'; +export { normalize } from './normalize'; diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/normalize.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/normalize.ts new file mode 100644 index 0000000000..83b83aaef2 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/normalize.ts @@ -0,0 +1,17 @@ +/* + * Copyright The OpenTelemetry Authors + * + * 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 + * + * https://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. + */ + +export { normalize } from 'path'; diff --git a/experimental/packages/opentelemetry-instrumentation/test/browser/noop-normalize.test.ts b/experimental/packages/opentelemetry-instrumentation/test/browser/noop-normalize.test.ts new file mode 100644 index 0000000000..aedcb975e7 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/test/browser/noop-normalize.test.ts @@ -0,0 +1,24 @@ +/* + * Copyright The OpenTelemetry Authors + * + * 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 + * + * https://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. + */ + +import * as assert from 'assert'; +import { normalize } from '../../src/platform/browser'; + +describe('noop-normalize', function () { + it('should not normalize input', function () { + assert.strictEqual(normalize('/tmp/foo/../bar'), '/tmp/foo/../bar'); + }); +});