Skip to content

Commit

Permalink
perf(deps): avoid semver where possible (#1309)
Browse files Browse the repository at this point in the history
avoid loading the semver dependency.
  • Loading branch information
TrySound authored Feb 3, 2021
1 parent ffbabfe commit 4c05cae
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 45 deletions.
8 changes: 0 additions & 8 deletions src/cls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

import {EventEmitter} from 'events';
import * as semver from 'semver';

import {AsyncHooksCLS} from './cls/async-hooks';
import {AsyncListenerCLS} from './cls/async-listener';
Expand All @@ -27,8 +26,6 @@ import {UNCORRELATED_ROOT_SPAN, DISABLED_ROOT_SPAN} from './span-data';
import {Trace, TraceSpan} from './trace';
import {Singleton} from './util';

const asyncHooksAvailable = semver.satisfies(process.version, '>=8');

export interface RealRootContext {
readonly span: TraceSpan;
readonly trace: Trace;
Expand Down Expand Up @@ -115,11 +112,6 @@ export class TraceCLS implements CLS<RootContext> {
constructor(config: TraceCLSConfig, private readonly logger: Logger) {
switch (config.mechanism) {
case TraceCLSMechanism.ASYNC_HOOKS:
if (!asyncHooksAvailable) {
throw new Error(
`CLS mechanism [${config.mechanism}] is not compatible with Node <8.`
);
}
this.CLSClass = AsyncHooksCLS;
this.rootSpanStackOffset = 4;
break;
Expand Down
16 changes: 11 additions & 5 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const filesLoadedBeforeTrace = Object.keys(require.cache);

// This file's top-level imports must not transitively depend on modules that
// do I/O, or continuation-local-storage will not work.
import * as semver from 'semver';
import {Config, defaultConfig} from './config';
import * as extend from 'extend';
import * as path from 'path';
Expand All @@ -28,6 +27,16 @@ import {TraceCLSMechanism} from './cls';
import {StackdriverTracer} from './trace-api';
import {TraceContextHeaderBehavior} from './tracing-policy';

if (process && process.version) {
const minNodeVersion = 10;
const major = Number(process.version.match(/v(\d+)/)![1]);
if (major < minNodeVersion) {
throw Error(
`trace-agent supports a minimum Node.js version of ${minNodeVersion}. Read our version support policy: https://github.com/googleapis/cloud-trace-nodejs#supported-nodejs-versions`
);
}
}

export {Config, PluginTypes};

let traceAgent: StackdriverTracer;
Expand Down Expand Up @@ -88,11 +97,8 @@ function initConfig(userConfig: Forceable<Config>): TopLevelConfig {
const getInternalClsMechanism = (clsMechanism: string): TraceCLSMechanism => {
// If the CLS mechanism is set to auto-determined, decide now
// what it should be.
const ahAvailable = semver.satisfies(process.version, '>=8');
if (clsMechanism === 'auto') {
return ahAvailable
? TraceCLSMechanism.ASYNC_HOOKS
: TraceCLSMechanism.ASYNC_LISTENER;
return TraceCLSMechanism.ASYNC_HOOKS;
}
return clsMechanism as TraceCLSMechanism;
};
Expand Down
54 changes: 22 additions & 32 deletions src/plugins/plugin-http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import * as httpModule from 'http';
import {Agent, ClientRequest, ClientRequestArgs, request} from 'http';
import * as httpsModule from 'https';
import * as semver from 'semver';
import * as shimmer from 'shimmer';
import {URL, UrlWithStringQuery} from 'url';

Expand All @@ -31,12 +30,7 @@ const ERR_HTTP_HEADERS_SENT_MSG = "Can't set headers after they are sent.";
// URL is used for type checking, but doesn't exist in Node <7.
// This function works around that.
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const isURL = semver.satisfies(process.version, '>=7')
? // eslint-disable-next-line @typescript-eslint/no-explicit-any
(value: any): value is URL => value instanceof URL
: // eslint-disable-next-line
(value: any): value is URL => false;
// tslint:enable:no-any
const isURL = (value: any): value is URL => value instanceof URL;

function getSpanName(options: ClientRequestArgs | URL) {
// c.f. _http_client.js ClientRequest constructor
Expand Down Expand Up @@ -268,28 +262,26 @@ function patchHttp(http: HttpModule, api: Tracer) {
return makeRequestTrace('http:', request, api);
});

if (semver.satisfies(process.version, '>=8.0.0')) {
// http.get in Node 8 calls the private copy of request rather than the one
// we have patched on module.export, so patch get as well.
shimmer.wrap(http, 'get', (): typeof http.get => {
// Re-implement http.get. This needs to be done (instead of using
// makeRequestTrace to patch it) because we need to set the trace
// context header before the returned ClientRequest is ended.
// The Node.js docs state that the only differences between request and
// get are that (1) get defaults to the HTTP GET method and (2) the
// returned request object is ended immediately.
// The former is already true (at least in supported Node versions up to
// v9), so we simply follow the latter.
// Ref:
// https://nodejs.org/dist/latest/docs/api/http.html#http_http_get_options_callback
return function getTrace(this: never) {
// eslint-disable-next-line prefer-rest-params
const req = http.request.apply(this, arguments);
req.end();
return req;
};
});
}
// http.get in Node 8 calls the private copy of request rather than the one
// we have patched on module.export, so patch get as well.
shimmer.wrap(http, 'get', (): typeof http.get => {
// Re-implement http.get. This needs to be done (instead of using
// makeRequestTrace to patch it) because we need to set the trace
// context header before the returned ClientRequest is ended.
// The Node.js docs state that the only differences between request and
// get are that (1) get defaults to the HTTP GET method and (2) the
// returned request object is ended immediately.
// The former is already true (at least in supported Node versions up to
// v9), so we simply follow the latter.
// Ref:
// https://nodejs.org/dist/latest/docs/api/http.html#http_http_get_options_callback
return function getTrace(this: never) {
// eslint-disable-next-line prefer-rest-params
const req = http.request.apply(this, arguments);
req.end();
return req;
};
});
}

// https.get depends on Node http internals in 8.9.0 and 9+ instead of the
Expand All @@ -310,9 +302,7 @@ function patchHttps(https: HttpsModule, api: Tracer) {

function unpatchHttp(http: HttpModule) {
shimmer.unwrap(http, 'request');
if (semver.satisfies(process.version, '>=8.0.0')) {
shimmer.unwrap(http, 'get');
}
shimmer.unwrap(http, 'get');
}

function unpatchHttps(https: HttpsModule) {
Expand Down

0 comments on commit 4c05cae

Please sign in to comment.