Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(resources): align exported names in different environments #2739

Merged
merged 2 commits into from
Jan 29, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions doc/development-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
24 changes: 24 additions & 0 deletions packages/opentelemetry-resources/karma.conf.js
Original file line number Diff line number Diff line change
@@ -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
}))
};
15 changes: 14 additions & 1 deletion packages/opentelemetry-resources/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,20 @@

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
* and being instrumented from the NodeJS Process module.
*/
class ProcessDetector implements Detector {
async detect(config?: ResourceDetectionConfig): Promise<Resource> {
// 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 || '',
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry-resources/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,4 @@ export * from './Resource';
export * from './platform';
export * from './types';
export * from './config';
export * from './detectors';
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,3 @@

export * from './default-service-name';
export * from './detect-resources';
export * from './detectors';
13 changes: 12 additions & 1 deletion packages/opentelemetry-resources/test/Resource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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]);
Expand All @@ -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');
});
});
});
Original file line number Diff line number Diff line change
@@ -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);
});
});
});
Original file line number Diff line number Diff line change
@@ -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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
Expand Down
25 changes: 25 additions & 0 deletions packages/opentelemetry-resources/test/index-webpack.ts
Original file line number Diff line number Diff line change
@@ -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);
}
33 changes: 33 additions & 0 deletions packages/opentelemetry-resources/test/util.ts
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down