From bbc1811b02df1194aa50dec590c05339bb633774 Mon Sep 17 00:00:00 2001 From: Marc Hassan Date: Thu, 20 Oct 2022 15:51:42 -0400 Subject: [PATCH 1/3] feat(instrumentation): implement `require-in-the-middle` singleton (#3161) Co-authored-by: Daniel Dyla Co-authored-by: Rauno Viskus --- experimental/CHANGELOG.md | 1 + .../src/platform/node/ModuleNameTrie.ts | 86 ++++++++++++ .../node/RequireInTheMiddleSingleton.ts | 111 +++++++++++++++ .../src/platform/node/instrumentation.ts | 10 +- .../test/node/ModuleNameTrie.test.ts | 68 ++++++++++ .../node/RequireInTheMiddleSingleton.test.ts | 126 ++++++++++++++++++ 6 files changed, 397 insertions(+), 5 deletions(-) create mode 100644 experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts create mode 100644 experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts create mode 100644 experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts create mode 100644 experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index 688384d220..f9ead87184 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) +* feat(instrumentation): implement `require-in-the-middle` singleton [#3161](https://github.com/open-telemetry/opentelemetry-js/pull/3161) @mhassan1 * feat(sdk-node): configure trace exporter with environment variables [#3143](https://github.com/open-telemetry/opentelemetry-js/pull/3143) @svetlanabrennan * feat: enable tree shaking [#3329](https://github.com/open-telemetry/opentelemetry-js/pull/3329) @pkanal * feat(prometheus): serialize resource as target_info gauge [#3300](https://github.com/open-telemetry/opentelemetry-js/pull/3300) @pichlermarc diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts new file mode 100644 index 0000000000..3230fea99c --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/ModuleNameTrie.ts @@ -0,0 +1,86 @@ +/* + * 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 type { Hooked } from './RequireInTheMiddleSingleton'; + +export const ModuleNameSeparator = '/'; + +/** + * Node in a `ModuleNameTrie` + */ +class ModuleNameTrieNode { + hooks: Array<{ hook: Hooked, insertedId: number }> = []; + children: Map = new Map(); +} + +/** + * Trie containing nodes that represent a part of a module name (i.e. the parts separated by forward slash) + */ +export class ModuleNameTrie { + private _trie: ModuleNameTrieNode = new ModuleNameTrieNode(); + private _counter: number = 0; + + /** + * Insert a module hook into the trie + * + * @param {Hooked} hook Hook + */ + insert(hook: Hooked) { + let trieNode = this._trie; + + for (const moduleNamePart of hook.moduleName.split(ModuleNameSeparator)) { + let nextNode = trieNode.children.get(moduleNamePart); + if (!nextNode) { + nextNode = new ModuleNameTrieNode(); + trieNode.children.set(moduleNamePart, nextNode); + } + trieNode = nextNode; + } + trieNode.hooks.push({ hook, insertedId: this._counter++ }); + } + + /** + * Search for matching hooks in the trie + * + * @param {string} moduleName Module name + * @param {boolean} maintainInsertionOrder Whether to return the results in insertion order + * @returns {Hooked[]} Matching hooks + */ + search(moduleName: string, { maintainInsertionOrder }: { maintainInsertionOrder?: boolean } = {}): Hooked[] { + let trieNode = this._trie; + const results: ModuleNameTrieNode['hooks'] = []; + + for (const moduleNamePart of moduleName.split(ModuleNameSeparator)) { + const nextNode = trieNode.children.get(moduleNamePart); + if (!nextNode) { + break; + } + results.push(...nextNode.hooks); + trieNode = nextNode; + } + + if (results.length === 0) { + return []; + } + if (results.length === 1) { + return [results[0].hook]; + } + if (maintainInsertionOrder) { + results.sort((a, b) => a.insertedId - b.insertedId); + } + return results.map(({ hook }) => hook); + } +} diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts new file mode 100644 index 0000000000..812db52b68 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/RequireInTheMiddleSingleton.ts @@ -0,0 +1,111 @@ +/* + * 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 RequireInTheMiddle from 'require-in-the-middle'; +import * as path from 'path'; +import { ModuleNameTrie, ModuleNameSeparator } from './ModuleNameTrie'; + +export type Hooked = { + moduleName: string + onRequire: RequireInTheMiddle.OnRequireFn +}; + +/** + * Whether Mocha is running in this process + * Inspired by https://github.com/AndreasPizsa/detect-mocha + * + * @type {boolean} + */ +const isMocha = ['afterEach','after','beforeEach','before','describe','it'].every(fn => { + // @ts-expect-error TS7053: Element implicitly has an 'any' type + return typeof global[fn] === 'function'; +}); + +/** + * Singleton class for `require-in-the-middle` + * Allows instrumentation plugins to patch modules with only a single `require` patch + * WARNING: Because this class will create its own `require-in-the-middle` (RITM) instance, + * we should minimize the number of new instances of this class. + * Multiple instances of `@opentelemetry/instrumentation` (e.g. multiple versions) in a single process + * will result in multiple instances of RITM, which will have an impact + * on the performance of instrumentation hooks being applied. + */ +export class RequireInTheMiddleSingleton { + private _moduleNameTrie: ModuleNameTrie = new ModuleNameTrie(); + private static _instance?: RequireInTheMiddleSingleton; + + private constructor() { + this._initialize(); + } + + private _initialize() { + RequireInTheMiddle( + // Intercept all `require` calls; we will filter the matching ones below + null, + { internals: true }, + (exports, name, basedir) => { + // For internal files on Windows, `name` will use backslash as the path separator + const normalizedModuleName = normalizePathSeparators(name); + + const matches = this._moduleNameTrie.search(normalizedModuleName, { maintainInsertionOrder: true }); + + for (const { onRequire } of matches) { + exports = onRequire(exports, name, basedir); + } + + return exports; + } + ); + } + + /** + * Register a hook with `require-in-the-middle` + * + * @param {string} moduleName Module name + * @param {RequireInTheMiddle.OnRequireFn} onRequire Hook function + * @returns {Hooked} Registered hook + */ + register(moduleName: string, onRequire: RequireInTheMiddle.OnRequireFn): Hooked { + const hooked = { moduleName, onRequire }; + this._moduleNameTrie.insert(hooked); + return hooked; + } + + /** + * Get the `RequireInTheMiddleSingleton` singleton + * + * @returns {RequireInTheMiddleSingleton} Singleton of `RequireInTheMiddleSingleton` + */ + static getInstance(): RequireInTheMiddleSingleton { + // Mocha runs all test suites in the same process + // This prevents test suites from sharing a singleton + if (isMocha) return new RequireInTheMiddleSingleton(); + + return this._instance = this._instance ?? new RequireInTheMiddleSingleton(); + } +} + +/** + * Normalize the path separators to forward slash in a module name or path + * + * @param {string} moduleNameOrPath Module name or path + * @returns {string} Normalized module name or path + */ +function normalizePathSeparators(moduleNameOrPath: string): string { + return path.sep !== ModuleNameSeparator + ? moduleNameOrPath.split(path.sep).join(ModuleNameSeparator) + : moduleNameOrPath; +} diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index 5bc0921778..d80985431c 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -16,9 +16,9 @@ import * as types from '../../types'; import * as path from 'path'; -import * as RequireInTheMiddle from 'require-in-the-middle'; import { satisfies } from 'semver'; import { InstrumentationAbstract } from '../../instrumentation'; +import { RequireInTheMiddleSingleton, Hooked } from './RequireInTheMiddleSingleton'; import { InstrumentationModuleDefinition } from './types'; import { diag } from '@opentelemetry/api'; @@ -29,7 +29,8 @@ export abstract class InstrumentationBase extends InstrumentationAbstract implements types.Instrumentation { private _modules: InstrumentationModuleDefinition[]; - private _hooks: RequireInTheMiddle.Hooked[] = []; + private _hooks: Hooked[] = []; + private _requireInTheMiddleSingleton: RequireInTheMiddleSingleton = RequireInTheMiddleSingleton.getInstance(); private _enabled = false; constructor( @@ -160,9 +161,8 @@ export abstract class InstrumentationBase this._warnOnPreloadedModules(); for (const module of this._modules) { this._hooks.push( - RequireInTheMiddle( - [module.name], - { internals: true }, + this._requireInTheMiddleSingleton.register( + module.name, (exports, name, baseDir) => { return this._onRequire( (module as unknown) as InstrumentationModuleDefinition< diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts new file mode 100644 index 0000000000..c3d72c89d7 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/test/node/ModuleNameTrie.test.ts @@ -0,0 +1,68 @@ +/* + * 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 { Hooked } from '../../src/platform/node/RequireInTheMiddleSingleton'; +import { ModuleNameTrie } from '../../src/platform/node/ModuleNameTrie'; + +describe('ModuleNameTrie', () => { + describe('search', () => { + const trie = new ModuleNameTrie(); + const inserts = [ + { moduleName: 'a', onRequire: () => {} }, + { moduleName: 'a/b', onRequire: () => {} }, + { moduleName: 'a', onRequire: () => {} }, + { moduleName: 'a/c', onRequire: () => {} }, + { moduleName: 'd', onRequire: () => {} } + ] as Hooked[]; + inserts.forEach(trie.insert.bind(trie)); + + it('should return a list of exact matches (no results)', () => { + assert.deepEqual(trie.search('e'), []); + }); + + it('should return a list of exact matches (one result)', () => { + assert.deepEqual(trie.search('d'), [inserts[4]]); + }); + + it('should return a list of exact matches (more than one result)', () => { + assert.deepEqual(trie.search('a'), [ + inserts[0], + inserts[2] + ]); + }); + + describe('maintainInsertionOrder = false', () => { + it('should return a list of matches in prefix order', () => { + assert.deepEqual(trie.search('a/b'), [ + inserts[0], + inserts[2], + inserts[1] + ]); + }); + }); + + describe('maintainInsertionOrder = true', () => { + it('should return a list of matches in insertion order', () => { + assert.deepEqual(trie.search('a/b', { maintainInsertionOrder: true }), [ + inserts[0], + inserts[1], + inserts[2] + ]); + }); + }); + }); +}); diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts new file mode 100644 index 0000000000..724dced720 --- /dev/null +++ b/experimental/packages/opentelemetry-instrumentation/test/node/RequireInTheMiddleSingleton.test.ts @@ -0,0 +1,126 @@ +/* + * 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 * as sinon from 'sinon'; +import * as path from 'path'; +import * as RequireInTheMiddle from 'require-in-the-middle'; +import { RequireInTheMiddleSingleton } from '../../src/platform/node/RequireInTheMiddleSingleton'; + +const requireInTheMiddleSingleton = RequireInTheMiddleSingleton.getInstance(); + +type AugmentedExports = { + __ritmOnRequires?: string[] +}; + +const makeOnRequiresStub = (label: string): sinon.SinonStub => sinon.stub().callsFake(((exports: AugmentedExports) => { + exports.__ritmOnRequires ??= []; + exports.__ritmOnRequires.push(label); + return exports; +}) as RequireInTheMiddle.OnRequireFn); + +describe('RequireInTheMiddleSingleton', () => { + describe('register', () => { + const onRequireFsStub = makeOnRequiresStub('fs'); + const onRequireFsPromisesStub = makeOnRequiresStub('fs-promises'); + const onRequireCodecovStub = makeOnRequiresStub('codecov'); + const onRequireCodecovLibStub = makeOnRequiresStub('codecov-lib'); + const onRequireCpxStub = makeOnRequiresStub('cpx'); + const onRequireCpxLibStub = makeOnRequiresStub('cpx-lib'); + + before(() => { + requireInTheMiddleSingleton.register('fs', onRequireFsStub); + requireInTheMiddleSingleton.register('fs/promises', onRequireFsPromisesStub); + requireInTheMiddleSingleton.register('codecov', onRequireCodecovStub); + requireInTheMiddleSingleton.register('codecov/lib/codecov.js', onRequireCodecovLibStub); + requireInTheMiddleSingleton.register('cpx', onRequireCpxStub); + requireInTheMiddleSingleton.register('cpx/lib/copy-sync.js', onRequireCpxLibStub); + }); + + beforeEach(() => { + onRequireFsStub.resetHistory(); + onRequireFsPromisesStub.resetHistory(); + onRequireCodecovStub.resetHistory(); + onRequireCodecovLibStub.resetHistory(); + onRequireCpxStub.resetHistory(); + onRequireCpxLibStub.resetHistory(); + }); + + it('should return a hooked object', () => { + const moduleName = 'm'; + const onRequire = makeOnRequiresStub('m'); + const hooked = requireInTheMiddleSingleton.register(moduleName, onRequire); + assert.deepStrictEqual(hooked, { moduleName, onRequire }); + }); + + describe('core module', () => { + describe('AND module name matches', () => { + it('should call `onRequire`', () => { + const exports = require('fs'); + assert.deepStrictEqual(exports.__ritmOnRequires, ['fs']); + sinon.assert.calledOnceWithExactly(onRequireFsStub, exports, 'fs', undefined); + sinon.assert.notCalled(onRequireFsPromisesStub); + }); + }); + describe('AND module name does not match', () => { + it('should not call `onRequire`', () => { + const exports = require('crypto'); + assert.equal(exports.__ritmOnRequires, undefined); + sinon.assert.notCalled(onRequireFsStub); + }); + }); + }); + + describe('core module with sub-path', () => { + describe('AND module name matches', () => { + it('should call `onRequire`', () => { + const exports = require('fs/promises'); + assert.deepStrictEqual(exports.__ritmOnRequires, ['fs', 'fs-promises']); + sinon.assert.calledOnceWithExactly(onRequireFsPromisesStub, exports, 'fs/promises', undefined); + sinon.assert.calledOnceWithMatch(onRequireFsStub, { __ritmOnRequires: ['fs', 'fs-promises'] }, 'fs/promises', undefined); + }); + }); + }); + + describe('non-core module', () => { + describe('AND module name matches', () => { + const baseDir = path.dirname(require.resolve('codecov')); + const modulePath = path.join('codecov', 'lib', 'codecov.js'); + it('should call `onRequire`', () => { + const exports = require('codecov'); + assert.deepStrictEqual(exports.__ritmOnRequires, ['codecov']); + sinon.assert.calledWithExactly(onRequireCodecovStub, exports, 'codecov', baseDir); + sinon.assert.calledWithMatch(onRequireCodecovStub, { __ritmOnRequires: ['codecov', 'codecov-lib'] }, modulePath, baseDir); + sinon.assert.calledWithMatch(onRequireCodecovLibStub, { __ritmOnRequires: ['codecov', 'codecov-lib'] }, modulePath, baseDir); + }).timeout(30000); + }); + }); + + describe('non-core module with sub-path', () => { + describe('AND module name matches', () => { + const baseDir = path.resolve(path.dirname(require.resolve('cpx')), '..'); + const modulePath = path.join('cpx', 'lib', 'copy-sync.js'); + it('should call `onRequire`', () => { + const exports = require('cpx/lib/copy-sync'); + assert.deepStrictEqual(exports.__ritmOnRequires, ['cpx', 'cpx-lib']); + sinon.assert.calledWithMatch(onRequireCpxStub, { __ritmOnRequires: ['cpx', 'cpx-lib'] }, modulePath, baseDir); + sinon.assert.calledWithExactly(onRequireCpxStub, exports, modulePath, baseDir); + sinon.assert.calledWithExactly(onRequireCpxLibStub, exports, modulePath, baseDir); + }); + }); + }); + }); +}); From 292a53df665be48c1425eca371397c9f00ae9e44 Mon Sep 17 00:00:00 2001 From: Hector Hernandez <39923391+hectorhdzg@users.noreply.github.com> Date: Thu, 20 Oct 2022 18:56:29 -0700 Subject: [PATCH 2/3] feat: add tracing suppresing for Metrics Export (#3332) --- experimental/CHANGELOG.md | 1 + .../export/PeriodicExportingMetricReader.ts | 21 +++------ packages/opentelemetry-core/src/index.ts | 4 ++ .../src/internal/exporter.ts | 38 ++++++++++++++++ .../test/internal/exporter.test.ts | 43 +++++++++++++++++++ .../src/export/SimpleSpanProcessor.ts | 30 ++++++------- 6 files changed, 108 insertions(+), 29 deletions(-) create mode 100644 packages/opentelemetry-core/src/internal/exporter.ts create mode 100644 packages/opentelemetry-core/test/internal/exporter.test.ts diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index f9ead87184..b25c11fb3c 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to experimental packages in this project will be documented ### :rocket: (Enhancement) +* feat(metrics-sdk): Add tracing suppresing for Metrics Export [#3332](https://github.com/open-telemetry/opentelemetry-js/pull/3332) @hectorhdzg * feat(instrumentation): implement `require-in-the-middle` singleton [#3161](https://github.com/open-telemetry/opentelemetry-js/pull/3161) @mhassan1 * feat(sdk-node): configure trace exporter with environment variables [#3143](https://github.com/open-telemetry/opentelemetry-js/pull/3143) @svetlanabrennan * feat: enable tree shaking [#3329](https://github.com/open-telemetry/opentelemetry-js/pull/3329) @pkanal diff --git a/experimental/packages/opentelemetry-sdk-metrics/src/export/PeriodicExportingMetricReader.ts b/experimental/packages/opentelemetry-sdk-metrics/src/export/PeriodicExportingMetricReader.ts index e4f78d1cf0..2a686181cc 100644 --- a/experimental/packages/opentelemetry-sdk-metrics/src/export/PeriodicExportingMetricReader.ts +++ b/experimental/packages/opentelemetry-sdk-metrics/src/export/PeriodicExportingMetricReader.ts @@ -16,6 +16,7 @@ import * as api from '@opentelemetry/api'; import { + internal, ExportResultCode, globalErrorHandler, unrefTimer @@ -85,20 +86,12 @@ export class PeriodicExportingMetricReader extends MetricReader { api.diag.error('PeriodicExportingMetricReader: metrics collection errors', ...errors); } - return new Promise((resolve, reject) => { - this._exporter.export(resourceMetrics, result => { - if (result.code !== ExportResultCode.SUCCESS) { - reject( - result.error ?? - new Error( - `PeriodicExportingMetricReader: metrics export failed (error ${result.error})` - ) - ); - } else { - resolve(); - } - }); - }); + const result = await internal._export(this._exporter, resourceMetrics); + if (result.code !== ExportResultCode.SUCCESS) { + throw new Error( + `PeriodicExportingMetricReader: metrics export failed (error ${result.error})` + ); + } } protected override onInitialized(): void { diff --git a/packages/opentelemetry-core/src/index.ts b/packages/opentelemetry-core/src/index.ts index fd8d3c6778..6c0834fe0f 100644 --- a/packages/opentelemetry-core/src/index.ts +++ b/packages/opentelemetry-core/src/index.ts @@ -42,3 +42,7 @@ export * from './utils/url'; export * from './utils/wrap'; export * from './utils/callback'; export * from './version'; +import { _export } from './internal/exporter'; +export const internal = { + _export +}; diff --git a/packages/opentelemetry-core/src/internal/exporter.ts b/packages/opentelemetry-core/src/internal/exporter.ts new file mode 100644 index 0000000000..a489b35eac --- /dev/null +++ b/packages/opentelemetry-core/src/internal/exporter.ts @@ -0,0 +1,38 @@ +/* + * 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 { context } from '@opentelemetry/api'; +import { ExportResult } from '../ExportResult'; +import { suppressTracing } from '../trace/suppress-tracing'; + +export interface Exporter { + export(arg: T, resultCallback: (result: ExportResult) => void): void; +} + +/** +* @internal +* Shared functionality used by Exporters while exporting data, including suppresion of Traces. +*/ +export function _export(exporter: Exporter, arg: T): Promise { + return new Promise(resolve => { + // prevent downstream exporter calls from generating spans + context.with(suppressTracing(context.active()), () => { + exporter.export(arg, (result: ExportResult) => { + resolve(result); + }); + }); + }); +} diff --git a/packages/opentelemetry-core/test/internal/exporter.test.ts b/packages/opentelemetry-core/test/internal/exporter.test.ts new file mode 100644 index 0000000000..1ca256d659 --- /dev/null +++ b/packages/opentelemetry-core/test/internal/exporter.test.ts @@ -0,0 +1,43 @@ +/* + * 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 * as sinon from 'sinon'; +import { ExportResult, ExportResultCode } from '../../src'; +import * as suppress from '../../src/trace/suppress-tracing'; +import { _export } from '../../src/internal/exporter'; + +describe('exporter', () => { + const sandbox = sinon.createSandbox(); + + afterEach(() => { + sandbox.restore(); + }); + + class TestExporter { + export(arg: any, resultCallback: (result: ExportResult) => void) { + resultCallback({ code: ExportResultCode.SUCCESS }); + } + } + + it('_export should suppress tracing', async () => { + const suppressSpy = sandbox.spy(suppress, 'suppressTracing'); + const exporter = new TestExporter(); + const result = await _export(exporter, ['Test1']); + assert.strictEqual(result.code, ExportResultCode.SUCCESS); + assert.ok(suppressSpy.calledOnce); + }); +}); diff --git a/packages/opentelemetry-sdk-trace-base/src/export/SimpleSpanProcessor.ts b/packages/opentelemetry-sdk-trace-base/src/export/SimpleSpanProcessor.ts index c775bdf6d4..a510ad02a6 100644 --- a/packages/opentelemetry-sdk-trace-base/src/export/SimpleSpanProcessor.ts +++ b/packages/opentelemetry-sdk-trace-base/src/export/SimpleSpanProcessor.ts @@ -14,12 +14,13 @@ * limitations under the License. */ -import { context, Context, TraceFlags } from '@opentelemetry/api'; +import { Context, TraceFlags } from '@opentelemetry/api'; import { + internal, ExportResultCode, globalErrorHandler, - suppressTracing, BindOnceFuture, + ExportResult } from '@opentelemetry/core'; import { Span } from '../Span'; import { SpanProcessor } from '../SpanProcessor'; @@ -45,7 +46,7 @@ export class SimpleSpanProcessor implements SpanProcessor { } // does nothing. - onStart(_span: Span, _parentContext: Context): void {} + onStart(_span: Span, _parentContext: Context): void { } onEnd(span: ReadableSpan): void { if (this._shutdownOnce.isCalled) { @@ -56,18 +57,17 @@ export class SimpleSpanProcessor implements SpanProcessor { return; } - // prevent downstream exporter calls from generating spans - context.with(suppressTracing(context.active()), () => { - this._exporter.export([span], result => { - if (result.code !== ExportResultCode.SUCCESS) { - globalErrorHandler( - result.error ?? - new Error( - `SimpleSpanProcessor: span export failed (status ${result})` - ) - ); - } - }); + internal._export(this._exporter, [span]).then((result: ExportResult) => { + if (result.code !== ExportResultCode.SUCCESS) { + globalErrorHandler( + result.error ?? + new Error( + `SimpleSpanProcessor: span export failed (status ${result})` + ) + ); + } + }).catch(error => { + globalErrorHandler(error); }); } From 30a81bdcd7243db1764d7a9f214f861f92f4ed45 Mon Sep 17 00:00:00 2001 From: Siim Kallas Date: Fri, 21 Oct 2022 12:34:29 +0300 Subject: [PATCH 3/3] chore: fix grpc and proto-loader deps (#3337) * chore: fix grpc and proto-looader deps * doc: update changelog --- experimental/CHANGELOG.md | 1 + experimental/packages/exporter-trace-otlp-grpc/package.json | 4 ++-- experimental/packages/exporter-trace-otlp-proto/package.json | 1 - .../opentelemetry-exporter-metrics-otlp-grpc/package.json | 4 ++-- .../opentelemetry-exporter-metrics-otlp-proto/package.json | 1 - .../packages/opentelemetry-instrumentation-grpc/package.json | 4 ++-- experimental/packages/otlp-grpc-exporter-base/package.json | 4 ++-- experimental/packages/otlp-proto-exporter-base/package.json | 1 - 8 files changed, 9 insertions(+), 11 deletions(-) diff --git a/experimental/CHANGELOG.md b/experimental/CHANGELOG.md index b25c11fb3c..44e89e8133 100644 --- a/experimental/CHANGELOG.md +++ b/experimental/CHANGELOG.md @@ -13,6 +13,7 @@ All notable changes to experimental packages in this project will be documented * feat(sdk-node): configure trace exporter with environment variables [#3143](https://github.com/open-telemetry/opentelemetry-js/pull/3143) @svetlanabrennan * feat: enable tree shaking [#3329](https://github.com/open-telemetry/opentelemetry-js/pull/3329) @pkanal * feat(prometheus): serialize resource as target_info gauge [#3300](https://github.com/open-telemetry/opentelemetry-js/pull/3300) @pichlermarc +* deps: remove unused proto-loader dependencies and update grpc-js and proto-loader versions [#3337](https://github.com/open-telemetry/opentelemetry-js/pull/3337) @seemk ### :bug: (Bug Fix) diff --git a/experimental/packages/exporter-trace-otlp-grpc/package.json b/experimental/packages/exporter-trace-otlp-grpc/package.json index 1bd113f7cd..05ba15892c 100644 --- a/experimental/packages/exporter-trace-otlp-grpc/package.json +++ b/experimental/packages/exporter-trace-otlp-grpc/package.json @@ -48,6 +48,7 @@ }, "devDependencies": { "@babel/core": "7.16.0", + "@grpc/proto-loader": "^0.7.3", "@opentelemetry/api": "^1.0.0", "@opentelemetry/otlp-exporter-base": "0.33.0", "@types/mocha": "10.0.0", @@ -67,8 +68,7 @@ "@opentelemetry/api": "^1.0.0" }, "dependencies": { - "@grpc/grpc-js": "^1.5.9", - "@grpc/proto-loader": "^0.6.9", + "@grpc/grpc-js": "^1.7.1", "@opentelemetry/core": "1.7.0", "@opentelemetry/otlp-grpc-exporter-base": "0.33.0", "@opentelemetry/otlp-transformer": "0.33.0", diff --git a/experimental/packages/exporter-trace-otlp-proto/package.json b/experimental/packages/exporter-trace-otlp-proto/package.json index dda47143ca..b9a6927640 100644 --- a/experimental/packages/exporter-trace-otlp-proto/package.json +++ b/experimental/packages/exporter-trace-otlp-proto/package.json @@ -66,7 +66,6 @@ "@opentelemetry/api": "^1.0.0" }, "dependencies": { - "@grpc/proto-loader": "^0.6.9", "@opentelemetry/core": "1.7.0", "@opentelemetry/otlp-exporter-base": "0.33.0", "@opentelemetry/otlp-proto-exporter-base": "0.33.0", diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/package.json b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/package.json index f4031ccf1f..d7dc5a21a8 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/package.json +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-grpc/package.json @@ -48,6 +48,7 @@ }, "devDependencies": { "@babel/core": "7.16.0", + "@grpc/proto-loader": "^0.7.3", "@opentelemetry/api": "^1.0.0", "@opentelemetry/api-metrics": "0.33.0", "@types/mocha": "10.0.0", @@ -67,8 +68,7 @@ "@opentelemetry/api": "^1.0.0" }, "dependencies": { - "@grpc/grpc-js": "^1.5.9", - "@grpc/proto-loader": "^0.6.9", + "@grpc/grpc-js": "^1.7.1", "@opentelemetry/core": "1.7.0", "@opentelemetry/exporter-metrics-otlp-http": "0.33.0", "@opentelemetry/otlp-grpc-exporter-base": "0.33.0", diff --git a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/package.json b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/package.json index 0f36c0bebb..bad7943e6e 100644 --- a/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/package.json +++ b/experimental/packages/opentelemetry-exporter-metrics-otlp-proto/package.json @@ -67,7 +67,6 @@ "@opentelemetry/api": "^1.0.0" }, "dependencies": { - "@grpc/proto-loader": "0.6.9", "@opentelemetry/core": "1.7.0", "@opentelemetry/exporter-metrics-otlp-http": "0.33.0", "@opentelemetry/otlp-exporter-base": "0.33.0", diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/package.json b/experimental/packages/opentelemetry-instrumentation-grpc/package.json index eb1ed7c949..8d9f69ef1c 100644 --- a/experimental/packages/opentelemetry-instrumentation-grpc/package.json +++ b/experimental/packages/opentelemetry-instrumentation-grpc/package.json @@ -45,8 +45,8 @@ "access": "public" }, "devDependencies": { - "@grpc/grpc-js": "1.5.9", - "@grpc/proto-loader": "0.6.9", + "@grpc/grpc-js": "^1.7.1", + "@grpc/proto-loader": "^0.7.3", "@opentelemetry/api": "^1.0.0", "@opentelemetry/context-async-hooks": "1.7.0", "@opentelemetry/core": "1.7.0", diff --git a/experimental/packages/otlp-grpc-exporter-base/package.json b/experimental/packages/otlp-grpc-exporter-base/package.json index 1c17da3744..b164cec8dd 100644 --- a/experimental/packages/otlp-grpc-exporter-base/package.json +++ b/experimental/packages/otlp-grpc-exporter-base/package.json @@ -71,8 +71,8 @@ "@opentelemetry/api": "^1.0.0" }, "dependencies": { - "@grpc/grpc-js": "^1.5.9", - "@grpc/proto-loader": "^0.6.9", + "@grpc/grpc-js": "^1.7.1", + "@grpc/proto-loader": "^0.7.3", "@opentelemetry/core": "1.7.0", "@opentelemetry/otlp-exporter-base": "0.33.0" }, diff --git a/experimental/packages/otlp-proto-exporter-base/package.json b/experimental/packages/otlp-proto-exporter-base/package.json index f10673d9f3..aaec196080 100644 --- a/experimental/packages/otlp-proto-exporter-base/package.json +++ b/experimental/packages/otlp-proto-exporter-base/package.json @@ -63,7 +63,6 @@ "@opentelemetry/api": "^1.0.0" }, "dependencies": { - "@grpc/proto-loader": "^0.6.9", "@opentelemetry/core": "1.7.0", "@opentelemetry/otlp-exporter-base": "0.33.0", "protobufjs": "7.1.1"