From 7de76c1e4c290d73bdd3b867eccd173c633e6877 Mon Sep 17 00:00:00 2001 From: legendecas Date: Wed, 26 Jan 2022 12:02:27 +0800 Subject: [PATCH] fix(resources): align exported names in different environments --- doc/development-guide.md | 30 +++++++++++ .../opentelemetry-resources/karma.conf.js | 24 +++++++++ packages/opentelemetry-resources/package.json | 15 +++++- .../node => }/detectors/EnvDetector.ts | 9 ++-- .../node => }/detectors/ProcessDetector.ts | 9 +++- .../{platform/node => }/detectors/index.ts | 0 packages/opentelemetry-resources/src/index.ts | 1 + .../src/platform/node/index.ts | 1 - .../test/Resource.test.ts | 13 ++++- .../detectors/browser/EnvDetector.test.ts | 53 +++++++++++++++++++ .../detectors/browser/ProcessDetector.test.ts | 32 +++++++++++ .../detectors/{ => node}/EnvDetector.test.ts | 7 +-- .../{ => node}/ProcessDetector.test.ts | 7 +-- .../test/index-webpack.ts | 25 +++++++++ packages/opentelemetry-resources/test/util.ts | 33 ++++++++++++ .../test/util/resource-assertions.ts | 25 +++++++++ 16 files changed, 267 insertions(+), 17 deletions(-) create mode 100644 packages/opentelemetry-resources/karma.conf.js rename packages/opentelemetry-resources/src/{platform/node => }/detectors/EnvDetector.ts (97%) rename packages/opentelemetry-resources/src/{platform/node => }/detectors/ProcessDetector.ts (90%) rename packages/opentelemetry-resources/src/{platform/node => }/detectors/index.ts (100%) create mode 100644 packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts create mode 100644 packages/opentelemetry-resources/test/detectors/browser/ProcessDetector.test.ts rename packages/opentelemetry-resources/test/detectors/{ => node}/EnvDetector.test.ts (89%) rename packages/opentelemetry-resources/test/detectors/{ => node}/ProcessDetector.test.ts (89%) create mode 100644 packages/opentelemetry-resources/test/index-webpack.ts create mode 100644 packages/opentelemetry-resources/test/util.ts diff --git a/doc/development-guide.md b/doc/development-guide.md index 69071c9a056..f4dd02a63e6 100644 --- a/doc/development-guide.md +++ b/doc/development-guide.md @@ -62,3 +62,33 @@ npm run docs ``` The document will be available under `packages/opentelemetry-api/docs/out` path. + +## Platform conditional exports + +Universal packages are packages that can be used in both web browsers and +Node.js environment. These packages may be implemented on top of different +platform APIs to achieve the same goal. Like accessing the _global_ reference, +we have different preferred ways to do it: + +- In Node.js, we access the _global_ reference with `globalThis` or `global`: + +```js +/// packages/opentelemetry-core/src/platform/node/globalThis.ts +export const _globalThis = typeof globalThis === 'object' ? globalThis : global; +``` + +- In web browser, we access the _global_ reference with the following definition: + +```js +/// packages/opentelemetry-core/src/platform/browser/globalThis.ts +export const _globalThis: typeof globalThis = + typeof globalThis === 'object' ? globalThis : + typeof self === 'object' ? self : + typeof window === 'object' ? window : + typeof global === 'object' ? global : + {} as typeof globalThis; +``` + +Even though the implementation may differ, the exported names must be aligned. +It can be confusing if exported names present in one environment but not in the +others. diff --git a/packages/opentelemetry-resources/karma.conf.js b/packages/opentelemetry-resources/karma.conf.js new file mode 100644 index 00000000000..3019564a15b --- /dev/null +++ b/packages/opentelemetry-resources/karma.conf.js @@ -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 + * + * 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. + */ + +const karmaWebpackConfig = require('../../karma.webpack'); +const karmaBaseConfig = require('../../karma.base'); + +module.exports = (config) => { + config.set(Object.assign({}, karmaBaseConfig, { + webpack: karmaWebpackConfig + })) +}; diff --git a/packages/opentelemetry-resources/package.json b/packages/opentelemetry-resources/package.json index 6c650ef9aed..f0c3f71a313 100644 --- a/packages/opentelemetry-resources/package.json +++ b/packages/opentelemetry-resources/package.json @@ -16,9 +16,11 @@ "scripts": { "compile": "tsc --build tsconfig.all.json", "clean": "tsc --build --clean tsconfig.all.json", + "codecov:browser": "nyc report --reporter=json && codecov -f coverage/*.json -p ../../", "lint": "eslint . --ext .ts", "lint:fix": "eslint . --ext .ts --fix", "test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'", + "test:browser": "nyc karma start --single-run", "tdd": "npm run test -- --watch-extensions ts --watch", "codecov": "nyc report --reporter=json && codecov -f coverage/*.json -p ../../", "version": "node ../../scripts/version-update.js", @@ -59,14 +61,25 @@ "@types/mocha": "8.2.3", "@types/node": "14.17.33", "@types/sinon": "10.0.6", + "@types/webpack-env": "1.16.3", "codecov": "3.8.3", + "karma": "6.3.8", + "karma-chrome-launcher": "3.1.0", + "karma-coverage-istanbul-reporter": "3.0.3", + "karma-mocha": "2.0.1", + "karma-mocha-webworker": "1.3.0", + "karma-spec-reporter": "0.0.32", + "karma-webpack": "4.0.2", "mocha": "7.2.0", "nock": "13.0.11", "nyc": "15.1.0", "rimraf": "3.0.2", "sinon": "12.0.1", "ts-mocha": "8.0.0", - "typescript": "4.4.4" + "typescript": "4.4.4", + "webpack": "4.46.0", + "webpack-cli": "4.9.1", + "webpack-merge": "5.8.0" }, "peerDependencies": { "@opentelemetry/api": ">=1.0.0 <1.1.0" diff --git a/packages/opentelemetry-resources/src/platform/node/detectors/EnvDetector.ts b/packages/opentelemetry-resources/src/detectors/EnvDetector.ts similarity index 97% rename from packages/opentelemetry-resources/src/platform/node/detectors/EnvDetector.ts rename to packages/opentelemetry-resources/src/detectors/EnvDetector.ts index a053c4da47e..782f664a1ab 100644 --- a/packages/opentelemetry-resources/src/platform/node/detectors/EnvDetector.ts +++ b/packages/opentelemetry-resources/src/detectors/EnvDetector.ts @@ -17,12 +17,9 @@ import { diag } from '@opentelemetry/api'; import { getEnv } from '@opentelemetry/core'; import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; -import { - Detector, - Resource, - ResourceDetectionConfig, - ResourceAttributes, -} from '../../../'; +import { Resource } from '../Resource'; +import { Detector, ResourceAttributes } from '../types'; +import { ResourceDetectionConfig } from '../config'; /** * EnvDetector can be used to detect the presence of and create a Resource diff --git a/packages/opentelemetry-resources/src/platform/node/detectors/ProcessDetector.ts b/packages/opentelemetry-resources/src/detectors/ProcessDetector.ts similarity index 90% rename from packages/opentelemetry-resources/src/platform/node/detectors/ProcessDetector.ts rename to packages/opentelemetry-resources/src/detectors/ProcessDetector.ts index 1df2d9b65fa..12e9e296bfa 100644 --- a/packages/opentelemetry-resources/src/platform/node/detectors/ProcessDetector.ts +++ b/packages/opentelemetry-resources/src/detectors/ProcessDetector.ts @@ -16,8 +16,9 @@ import { diag } from '@opentelemetry/api'; import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; -import { Detector, Resource, ResourceDetectionConfig } from '../../../'; -import { ResourceAttributes } from '../../../types'; +import { Resource } from '../Resource'; +import { Detector, ResourceAttributes } from '../types'; +import { ResourceDetectionConfig } from '../config'; /** * ProcessDetector will be used to detect the resources related current process running @@ -25,6 +26,10 @@ import { ResourceAttributes } from '../../../types'; */ class ProcessDetector implements Detector { async detect(config?: ResourceDetectionConfig): Promise { + // Skip if not in Node.js environment. + if (typeof process !== 'object') { + return Resource.empty(); + } const processResource: ResourceAttributes = { [SemanticResourceAttributes.PROCESS_PID]: process.pid, [SemanticResourceAttributes.PROCESS_EXECUTABLE_NAME]: process.title || '', diff --git a/packages/opentelemetry-resources/src/platform/node/detectors/index.ts b/packages/opentelemetry-resources/src/detectors/index.ts similarity index 100% rename from packages/opentelemetry-resources/src/platform/node/detectors/index.ts rename to packages/opentelemetry-resources/src/detectors/index.ts diff --git a/packages/opentelemetry-resources/src/index.ts b/packages/opentelemetry-resources/src/index.ts index 656de7291e8..4cff4181e35 100644 --- a/packages/opentelemetry-resources/src/index.ts +++ b/packages/opentelemetry-resources/src/index.ts @@ -18,3 +18,4 @@ export * from './Resource'; export * from './platform'; export * from './types'; export * from './config'; +export * from './detectors'; diff --git a/packages/opentelemetry-resources/src/platform/node/index.ts b/packages/opentelemetry-resources/src/platform/node/index.ts index 00426f781ca..742d36add7d 100644 --- a/packages/opentelemetry-resources/src/platform/node/index.ts +++ b/packages/opentelemetry-resources/src/platform/node/index.ts @@ -16,4 +16,3 @@ export * from './default-service-name'; export * from './detect-resources'; -export * from './detectors'; diff --git a/packages/opentelemetry-resources/test/Resource.test.ts b/packages/opentelemetry-resources/test/Resource.test.ts index b2e656b3095..c3780591d68 100644 --- a/packages/opentelemetry-resources/test/Resource.test.ts +++ b/packages/opentelemetry-resources/test/Resource.test.ts @@ -18,6 +18,7 @@ import * as assert from 'assert'; import { SDK_INFO } from '@opentelemetry/core'; import { Resource } from '../src'; import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; +import { describeBrowser, describeNode } from './util'; describe('Resource', () => { const resource1 = new Resource({ @@ -100,7 +101,7 @@ describe('Resource', () => { }); }); - describe('.default()', () => { + describeNode('.default()', () => { it('should return a default resource', () => { const resource = Resource.default(); assert.strictEqual(resource.attributes[SemanticResourceAttributes.TELEMETRY_SDK_NAME], SDK_INFO[SemanticResourceAttributes.TELEMETRY_SDK_NAME]); @@ -109,4 +110,14 @@ describe('Resource', () => { assert.strictEqual(resource.attributes[SemanticResourceAttributes.SERVICE_NAME], `unknown_service:${process.argv0}`); }); }); + + describeBrowser('.default()', () => { + it('should return a default resource', () => { + const resource = Resource.default(); + assert.strictEqual(resource.attributes[SemanticResourceAttributes.TELEMETRY_SDK_NAME], SDK_INFO[SemanticResourceAttributes.TELEMETRY_SDK_NAME]); + assert.strictEqual(resource.attributes[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE], SDK_INFO[SemanticResourceAttributes.TELEMETRY_SDK_LANGUAGE]); + assert.strictEqual(resource.attributes[SemanticResourceAttributes.TELEMETRY_SDK_VERSION], SDK_INFO[SemanticResourceAttributes.TELEMETRY_SDK_VERSION]); + assert.strictEqual(resource.attributes[SemanticResourceAttributes.SERVICE_NAME], 'unknown_service'); + }); + }); }); diff --git a/packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts b/packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts new file mode 100644 index 00000000000..940b5b07c02 --- /dev/null +++ b/packages/opentelemetry-resources/test/detectors/browser/EnvDetector.test.ts @@ -0,0 +1,53 @@ +/* + * 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 { RAW_ENVIRONMENT } from '@opentelemetry/core'; +import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; +import { envDetector, Resource } from '../../../src'; +import { + assertEmptyResource, + assertWebEngineResource, +} from '../../util/resource-assertions'; +import { describeBrowser } from '../../util'; + +describeBrowser('envDetector() on web browser', () => { + describe('with valid env', () => { + before(() => { + (globalThis as typeof globalThis & RAW_ENVIRONMENT).OTEL_RESOURCE_ATTRIBUTES = + 'webengine.name="chromium",webengine.version="99",webengine.description="Chromium"'; + }); + + after(() => { + delete (globalThis as typeof globalThis & RAW_ENVIRONMENT).OTEL_RESOURCE_ATTRIBUTES; + }); + + it('should return resource information from environment variable', async () => { + const resource: Resource = await envDetector.detect(); + assertWebEngineResource(resource, { + [SemanticResourceAttributes.WEBENGINE_NAME]: 'chromium', + [SemanticResourceAttributes.WEBENGINE_VERSION]: '99', + [SemanticResourceAttributes.WEBENGINE_DESCRIPTION]: 'Chromium', + }); + }); + }); + + describe('with empty env', () => { + it('should return empty resource', async () => { + const resource: Resource = await envDetector.detect(); + assertEmptyResource(resource); + }); + }); +}); diff --git a/packages/opentelemetry-resources/test/detectors/browser/ProcessDetector.test.ts b/packages/opentelemetry-resources/test/detectors/browser/ProcessDetector.test.ts new file mode 100644 index 00000000000..4b69b479ebd --- /dev/null +++ b/packages/opentelemetry-resources/test/detectors/browser/ProcessDetector.test.ts @@ -0,0 +1,32 @@ +/* + * 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 sinon from 'sinon'; +import { processDetector, Resource } from '../../../src'; +import { + assertEmptyResource, +} from '../../util/resource-assertions'; +import { describeBrowser } from '../../util'; + +describeBrowser('processDetector() on web browser', () => { + afterEach(() => { + sinon.restore(); + }); + + it('should return empty resource', async () => { + const resource: Resource = await processDetector.detect(); + assertEmptyResource(resource); + }); +}); diff --git a/packages/opentelemetry-resources/test/detectors/EnvDetector.test.ts b/packages/opentelemetry-resources/test/detectors/node/EnvDetector.test.ts similarity index 89% rename from packages/opentelemetry-resources/test/detectors/EnvDetector.test.ts rename to packages/opentelemetry-resources/test/detectors/node/EnvDetector.test.ts index 64c1ff37567..2a2746e14ba 100644 --- a/packages/opentelemetry-resources/test/detectors/EnvDetector.test.ts +++ b/packages/opentelemetry-resources/test/detectors/node/EnvDetector.test.ts @@ -15,13 +15,14 @@ */ import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; -import { envDetector, Resource } from '../../src'; +import { envDetector, Resource } from '../../../src'; import { assertK8sResource, assertEmptyResource, -} from '../util/resource-assertions'; +} from '../../util/resource-assertions'; +import { describeNode } from '../../util'; -describe('envDetector()', () => { +describeNode('envDetector() on Node.js', () => { describe('with valid env', () => { before(() => { process.env.OTEL_RESOURCE_ATTRIBUTES = diff --git a/packages/opentelemetry-resources/test/detectors/ProcessDetector.test.ts b/packages/opentelemetry-resources/test/detectors/node/ProcessDetector.test.ts similarity index 89% rename from packages/opentelemetry-resources/test/detectors/ProcessDetector.test.ts rename to packages/opentelemetry-resources/test/detectors/node/ProcessDetector.test.ts index ff4d3fa7a7a..a76c728c299 100644 --- a/packages/opentelemetry-resources/test/detectors/ProcessDetector.test.ts +++ b/packages/opentelemetry-resources/test/detectors/node/ProcessDetector.test.ts @@ -14,13 +14,14 @@ * limitations under the License. */ import * as sinon from 'sinon'; -import { processDetector, Resource } from '../../src'; +import { processDetector, Resource } from '../../../src'; import { assertProcessResource, assertEmptyResource, -} from '../util/resource-assertions'; +} from '../../util/resource-assertions'; +import { describeNode } from '../../util'; -describe('processDetector()', () => { +describeNode('processDetector() on Node.js', () => { afterEach(() => { sinon.restore(); }); diff --git a/packages/opentelemetry-resources/test/index-webpack.ts b/packages/opentelemetry-resources/test/index-webpack.ts new file mode 100644 index 00000000000..dd9b144e0de --- /dev/null +++ b/packages/opentelemetry-resources/test/index-webpack.ts @@ -0,0 +1,25 @@ +/* + * 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. + */ + +{ + const testsContext = require.context('./', false, /test$/); + testsContext.keys().forEach(testsContext); +} + +{ + const testsContext = require.context('./detectors/browser', false, /test$/); + testsContext.keys().forEach(testsContext); +} diff --git a/packages/opentelemetry-resources/test/util.ts b/packages/opentelemetry-resources/test/util.ts new file mode 100644 index 00000000000..bd1fa7853c7 --- /dev/null +++ b/packages/opentelemetry-resources/test/util.ts @@ -0,0 +1,33 @@ +/* + * 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 { Suite } from 'mocha'; + +export function describeBrowser(title: string, fn: (this: Suite) => void) { + title = `Browser: ${title}`; + if (typeof process === 'object' && process.release?.name === 'node') { + return describe.skip(title, fn); + } + return describe(title, fn); +} + +export function describeNode(title: string, fn: (this: Suite) => void) { + title = `Node.js: ${title}`; + if (typeof process !== 'object' || process.release?.name !== 'node') { + return describe.skip(title, fn); + } + return describe(title, fn); +} diff --git a/packages/opentelemetry-resources/test/util/resource-assertions.ts b/packages/opentelemetry-resources/test/util/resource-assertions.ts index 9177aea703f..37fd429d478 100644 --- a/packages/opentelemetry-resources/test/util/resource-assertions.ts +++ b/packages/opentelemetry-resources/test/util/resource-assertions.ts @@ -296,6 +296,31 @@ export const assertProcessResource = ( } }; +export const assertWebEngineResource = (resource: Resource, validations: { + name?: string; + version?: string; + description?: string; +}) => { + if (validations.name) { + assert.strictEqual( + resource.attributes[SemanticResourceAttributes.WEBENGINE_NAME], + validations.name + ); + } + if (validations.version) { + assert.strictEqual( + resource.attributes[SemanticResourceAttributes.WEBENGINE_VERSION], + validations.version + ); + } + if (validations.description) { + assert.strictEqual( + resource.attributes[SemanticResourceAttributes.WEBENGINE_DESCRIPTION], + validations.description + ); + } +}; + /** * Test utility method to validate an empty resource *