Skip to content

Commit

Permalink
chore: remove checks for unsupported versions (#4341)
Browse files Browse the repository at this point in the history
* chore: remove checks for unsupported versions

* Remove final version checks

* Changelog

* Lint
  • Loading branch information
dyladan authored Dec 13, 2023
1 parent ea8bfa2 commit ba00147
Show file tree
Hide file tree
Showing 14 changed files with 25 additions and 97 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG_NEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@

### :house: (Internal)

* chore: remove checks for unsupported node versions [#4341](https://github.com/open-telemetry/opentelemetry-js/pull/4341) @dyladan

### :bug: (Bug Fix)
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,12 @@
"@protobuf-ts/runtime-rpc": "2.9.3",
"@types/mocha": "10.0.6",
"@types/node": "18.6.5",
"@types/semver": "7.5.6",
"@types/sinon": "10.0.20",
"codecov": "3.8.3",
"cross-var": "1.1.0",
"lerna": "6.6.2",
"mocha": "10.2.0",
"nyc": "15.1.0",
"semver": "7.5.4",
"sinon": "15.1.2",
"ts-mocha": "10.0.0",
"typescript": "4.4.4"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"@types/mocha": "10.0.6",
"@types/node": "18.6.5",
"@types/request-promise-native": "1.0.21",
"@types/semver": "7.5.6",
"@types/sinon": "10.0.20",
"@types/superagent": "4.1.24",
"axios": "1.5.1",
Expand All @@ -76,8 +75,7 @@
"dependencies": {
"@opentelemetry/core": "1.18.1",
"@opentelemetry/instrumentation": "0.45.1",
"@opentelemetry/semantic-conventions": "1.18.1",
"semver": "^7.5.2"
"@opentelemetry/semantic-conventions": "1.18.1"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/experimental/packages/opentelemetry-instrumentation-http",
"sideEffects": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import {
import type * as http from 'http';
import type * as https from 'https';
import { Socket } from 'net';
import * as semver from 'semver';
import * as url from 'url';
import {
Err,
Expand Down Expand Up @@ -398,10 +397,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
};

response.on('end', endHandler);
// See https://github.com/open-telemetry/opentelemetry-js/pull/3625#issuecomment-1475673533
if (semver.lt(process.version, '16.0.0')) {
response.on('close', endHandler);
}
response.on(errorMonitor, (error: Err) => {
this._diag.debug('outgoingRequest on error()', error);
if (responseFinished) {
Expand Down Expand Up @@ -609,18 +604,6 @@ export class HttpInstrumentation extends InstrumentationBase<Http> {
options,
extraOptions
);
/**
* Node 8's https module directly call the http one so to avoid creating
* 2 span for the same request we need to check that the protocol is correct
* See: https://github.com/nodejs/node/blob/v8.17.0/lib/https.js#L245
*/
if (
component === 'http' &&
semver.lt(process.version, '9.0.0') &&
optionsParsed.protocol === 'https:'
) {
return original.apply(this, [optionsParsed, ...args]);
}

if (
utils.isIgnored(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,7 @@ describe('Packages', () => {
},
].forEach(({ name, httpPackage }) => {
it(`should create a span for GET requests and add propagation headers by using ${name} package`, async () => {
if (process.versions.node.startsWith('12') && name === 'got') {
// got complains with nock and node version 12+
// > RequestError: The first argument must be one of type string, Buffer, ArrayBuffer, Array, or Array-like Object. Received type function
// so let's make a real call
nock.cleanAll();
nock.enableNetConnect();
} else {
nock.load(path.join(__dirname, '../', '/fixtures/google-http.json'));
}
nock.load(path.join(__dirname, '../', '/fixtures/google-http.json'));

const urlparsed = url.parse(
`${protocol}://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,10 @@ describe('Packages', () => {
},
].forEach(({ name, httpPackage }) => {
it(`should create a span for GET requests and add propagation headers by using ${name} package`, async () => {
if (process.versions.node.startsWith('12') && name === 'got') {
// got complains with nock and node version 12+
// > RequestError: The first argument must be one of type string, Buffer, ArrayBuffer, Array, or Array-like Object. Received type function
// so let's make a real call
nock.cleanAll();
nock.enableNetConnect();
} else {
nock.load(path.join(__dirname, '../', '/fixtures/google-https.json'));
}
nock.load(path.join(__dirname, '../', '/fixtures/google-https.json'));

const urlparsed = url.parse(
name === 'got' && process.versions.node.startsWith('12')
? // there is an issue with got 9.6 version and node 12 when redirecting so url above will not work
// https://github.com/nock/nock/pull/1551
// https://github.com/sindresorhus/got/commit/bf1aa5492ae2bc78cbbec6b7d764906fb156e6c2#diff-707a4781d57c42085155dcb27edb9ccbR258
// TODO: check if this is still the case when new version
'https://www.google.com'
: 'https://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8'
'https://www.google.com/search?q=axios&oq=axios&aqs=chrome.0.69i59l2j0l3j69i60.811j0j7&sourceid=chrome&ie=UTF-8'
);
const result = await httpPackage.get(urlparsed.href!);
if (!resHeaders) {
Expand Down
2 changes: 0 additions & 2 deletions experimental/packages/opentelemetry-sdk-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,12 @@
"@opentelemetry/exporter-jaeger": "1.18.1",
"@types/mocha": "10.0.6",
"@types/node": "18.6.5",
"@types/semver": "7.5.6",
"@types/sinon": "10.0.20",
"codecov": "3.8.3",
"cross-var": "1.1.0",
"lerna": "6.6.2",
"mocha": "10.2.0",
"nyc": "15.1.0",
"semver": "7.5.4",
"sinon": "15.1.2",
"ts-loader": "8.4.0",
"ts-mocha": "10.0.0",
Expand Down
14 changes: 3 additions & 11 deletions experimental/packages/opentelemetry-sdk-node/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ import {
metrics,
DiagConsoleLogger,
} from '@opentelemetry/api';
import {
AsyncHooksContextManager,
AsyncLocalStorageContextManager,
} from '@opentelemetry/context-async-hooks';
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
import { CompositePropagator } from '@opentelemetry/core';
import {
AggregationTemporality,
Expand All @@ -48,7 +45,6 @@ import {
IdGenerator,
} from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import * as semver from 'semver';
import * as Sinon from 'sinon';
import { NodeSDK } from '../src';
import { env } from 'process';
Expand All @@ -66,10 +62,6 @@ import {
LoggerProvider,
} from '@opentelemetry/sdk-logs';

const DefaultContextManager = semver.gte(process.version, '14.8.0')
? AsyncLocalStorageContextManager
: AsyncHooksContextManager;

describe('Node SDK', () => {
let ctxManager: any;
let propagator: any;
Expand Down Expand Up @@ -166,7 +158,7 @@ describe('Node SDK', () => {

assert.ok(
context['_getContextManager']().constructor.name ===
DefaultContextManager.name
AsyncLocalStorageContextManager.name
);
assert.ok(
propagation['_getGlobalPropagator']() instanceof CompositePropagator
Expand All @@ -191,7 +183,7 @@ describe('Node SDK', () => {

assert.ok(
context['_getContextManager']().constructor.name ===
DefaultContextManager.name
AsyncLocalStorageContextManager.name
);
assert.ok(
propagation['_getGlobalPropagator']() instanceof CompositePropagator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export function sendWithHttp<ExportItem, ServiceRequest>(
): void {
const exporterTimeout = collector.timeoutMillis;
const parsedUrl = new url.URL(collector.url);
const nodeVersion = Number(process.versions.node.split('.')[0]);
let retryTimer: ReturnType<typeof setTimeout>;
let req: http.ClientRequest;
let reqIsDestroyed = false;
Expand All @@ -63,8 +62,7 @@ export function sendWithHttp<ExportItem, ServiceRequest>(
const err = new OTLPExporterError('Request Timeout');
onError(err);
} else {
// req.abort() was deprecated since v14
nodeVersion >= 14 ? req.destroy() : req.abort();
req.destroy();
}
}, exporterTimeout);

Expand Down
3 changes: 1 addition & 2 deletions experimental/packages/shim-opencensus/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@
"@opentelemetry/core": "1.18.1",
"@opentelemetry/resources": "1.18.1",
"@opentelemetry/sdk-metrics": "1.18.1",
"require-in-the-middle": "^7.1.1",
"semver": "^7.5.2"
"require-in-the-middle": "^7.1.1"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/shim-opencensus",
"sideEffects": false
Expand Down
11 changes: 2 additions & 9 deletions experimental/packages/shim-opencensus/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,7 @@ import {
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import { ShimTracer } from '../src/ShimTracer';
import * as semver from 'semver';
import {
AsyncHooksContextManager,
AsyncLocalStorageContextManager,
} from '@opentelemetry/context-async-hooks';
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
import { Tracer, TracerProvider, context } from '@opentelemetry/api';

export async function withTestTracer(
Expand Down Expand Up @@ -61,10 +57,7 @@ export function setupNodeContextManager(
before: Mocha.HookFunction,
after: Mocha.HookFunction
) {
const ContextManager = semver.gte(process.version, '14.8.0')
? AsyncLocalStorageContextManager
: AsyncHooksContextManager;
const instance = new ContextManager();
const instance = new AsyncLocalStorageContextManager();
instance.enable();
before(() => context.setGlobalContextManager(instance));
after(() => context.disable());
Expand Down
4 changes: 1 addition & 3 deletions packages/opentelemetry-sdk-trace-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@
"@opentelemetry/semantic-conventions": "1.18.1",
"@types/mocha": "10.0.6",
"@types/node": "18.6.5",
"@types/semver": "7.5.6",
"@types/sinon": "10.0.20",
"codecov": "3.8.3",
"cross-var": "1.1.0",
Expand All @@ -69,8 +68,7 @@
"@opentelemetry/core": "1.18.1",
"@opentelemetry/propagator-b3": "1.18.1",
"@opentelemetry/propagator-jaeger": "1.18.1",
"@opentelemetry/sdk-trace-base": "1.18.1",
"semver": "^7.5.2"
"@opentelemetry/sdk-trace-base": "1.18.1"
},
"homepage": "https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-sdk-trace-node",
"sideEffects": false
Expand Down
11 changes: 2 additions & 9 deletions packages/opentelemetry-sdk-trace-node/src/NodeTracerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {
AsyncHooksContextManager,
AsyncLocalStorageContextManager,
} from '@opentelemetry/context-async-hooks';
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
import { B3Propagator, B3InjectEncoding } from '@opentelemetry/propagator-b3';
import {
BasicTracerProvider,
PROPAGATOR_FACTORY,
SDKRegistrationConfig,
} from '@opentelemetry/sdk-trace-base';
import * as semver from 'semver';
import { NodeTracerConfig } from './config';
import { JaegerPropagator } from '@opentelemetry/propagator-jaeger';

Expand Down Expand Up @@ -58,10 +54,7 @@ export class NodeTracerProvider extends BasicTracerProvider {

override register(config: SDKRegistrationConfig = {}): void {
if (config.contextManager === undefined) {
const ContextManager = semver.gte(process.version, '14.8.0')
? AsyncLocalStorageContextManager
: AsyncHooksContextManager;
config.contextManager = new ContextManager();
config.contextManager = new AsyncLocalStorageContextManager();
config.contextManager.enable();
}

Expand Down
20 changes: 9 additions & 11 deletions packages/opentelemetry-sdk-trace-node/test/registration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,9 @@ import {
trace,
ProxyTracerProvider,
} from '@opentelemetry/api';
import {
AsyncHooksContextManager,
AsyncLocalStorageContextManager,
} from '@opentelemetry/context-async-hooks';
import { AsyncLocalStorageContextManager } from '@opentelemetry/context-async-hooks';
import { CompositePropagator } from '@opentelemetry/core';
import { NodeTracerProvider } from '../src';
import * as semver from 'semver';

const assertInstanceOf = (actual: object, ExpectedInstance: Function) => {
assert.ok(
Expand All @@ -38,10 +34,6 @@ const assertInstanceOf = (actual: object, ExpectedInstance: Function) => {
);
};

const DefaultContextManager = semver.gte(process.version, '14.8.0')
? AsyncLocalStorageContextManager
: AsyncHooksContextManager;

describe('API registration', () => {
beforeEach(() => {
context.disable();
Expand All @@ -53,7 +45,10 @@ describe('API registration', () => {
const tracerProvider = new NodeTracerProvider();
tracerProvider.register();

assertInstanceOf(context['_getContextManager'](), DefaultContextManager);
assertInstanceOf(
context['_getContextManager'](),
AsyncLocalStorageContextManager
);
assertInstanceOf(
propagation['_getGlobalPropagator'](),
CompositePropagator
Expand Down Expand Up @@ -113,7 +108,10 @@ describe('API registration', () => {

assert.strictEqual(propagation['_getGlobalPropagator'](), propagator);

assertInstanceOf(context['_getContextManager'](), DefaultContextManager);
assertInstanceOf(
context['_getContextManager'](),
AsyncLocalStorageContextManager
);

const apiTracerProvider = trace.getTracerProvider() as ProxyTracerProvider;
assert.ok(apiTracerProvider.getDelegate() === tracerProvider);
Expand Down

0 comments on commit ba00147

Please sign in to comment.