From 0f8a6af71a0604825533e2498c71c0b37703089e Mon Sep 17 00:00:00 2001 From: MartenH <72463136+mhennoch@users.noreply.github.com> Date: Tue, 2 Apr 2024 13:48:43 +0300 Subject: [PATCH] Extract configureInstrumentations from tracing (#892) * Extract configureInstrumentations from tracing * Increse debug metrics test timeout * Extract setting default options also * Increase jobs * Fix example * Cleanup --- examples/express/tracer.js | 2 +- package.json | 4 +- src/instrument.ts | 12 ++-- src/instrumentations/graphql.ts | 4 +- src/instrumentations/http.ts | 9 +-- src/instrumentations/index.ts | 108 ++++++++++++++++++++++++++++- src/instrumentations/redis.ts | 4 +- src/logging/index.ts | 17 ++--- src/metrics/index.ts | 9 +-- src/profiling/index.ts | 8 +-- src/start.ts | 55 +++------------ src/tracing/index.ts | 51 +------------- src/tracing/options.ts | 4 +- test/api.test.ts | 6 +- test/instrumentation/redis.test.ts | 11 ++- test/logging.test.ts | 4 +- test/loginjection.bunyan.test.ts | 6 +- test/loginjection.pino.test.ts | 46 ++++++------ test/loginjection.winston.test.ts | 6 +- test/metrics.test.ts | 30 ++++---- test/options.test.ts | 7 +- test/propagation.test.ts | 25 ++++--- test/servertiming.test.ts | 44 +++++++----- test/start.test.ts | 20 +++--- test/tracing/tracing.test.ts | 53 +++++++++----- test/uri_parameter_capture.test.ts | 48 ++++++++----- 26 files changed, 345 insertions(+), 248 deletions(-) diff --git a/examples/express/tracer.js b/examples/express/tracer.js index a543de14..da806aa2 100644 --- a/examples/express/tracer.js +++ b/examples/express/tracer.js @@ -14,4 +14,4 @@ if (process.env.OTEL_LOG_LEVEL) { diag.setLogger(new DiagConsoleLogger(), DiagLogLevel[process.env.OTEL_LOG_LEVEL]); } -const tracer = require('@splunk/otel').startTracing(); +require('@splunk/otel').start(); diff --git a/package.json b/package.json index 1c2fee3f..0abc183b 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,8 @@ "lint": "eslint . --ext .ts", "lint:commits": "commitlint", "test": "npm run test:unit && npm run test:debug-metrics && npm run test:instrumentations", - "test:unit": "cross-env TEST_ALLOW_DOUBLE_START=y nyc ts-mocha --exclude 'test/instrumentation/external/**/*.test.ts' --exclude 'test/separate_process/*' --timeout 60s --parallel --jobs 4 -p tsconfig.json 'test/**/*.test.ts'", - "test:debug-metrics": "nyc --no-clean ts-mocha -p tsconfig.json 'test/separate_process/debug_metrics.test.ts'", + "test:unit": "cross-env TEST_ALLOW_DOUBLE_START=y nyc ts-mocha --exclude 'test/instrumentation/external/**/*.test.ts' --exclude 'test/separate_process/*' --timeout 60s --parallel --jobs 8 -p tsconfig.json 'test/**/*.test.ts'", + "test:debug-metrics": "nyc --no-clean ts-mocha --timeout 10000 -p tsconfig.json 'test/separate_process/debug_metrics.test.ts'", "test:instrumentations": "nyc ts-mocha --require test/instrumentation/external/setup.ts --jobs 1 'test/instrumentation/external/**/*.test.ts'", "prebuild:current": "node scripts/prebuild-current.js", "prebuild:os": "node scripts/prebuild-os.js", diff --git a/src/instrument.ts b/src/instrument.ts index 2aa5452e..cdc213e4 100644 --- a/src/instrument.ts +++ b/src/instrument.ts @@ -27,6 +27,7 @@ import { startMetrics } from './metrics'; import { startProfiling } from './profiling'; import { startTracing } from './tracing'; import { startLogging } from './logging'; +import { parseOptionsAndConfigureInstrumentations } from './instrumentations'; function boot() { const logLevel = parseLogLevel(getNonEmptyEnvVar('OTEL_LOG_LEVEL')); @@ -46,21 +47,24 @@ function boot() { } } + const { tracingOptions, loggingOptions, profilingOptions, metricsOptions } = + parseOptionsAndConfigureInstrumentations(); + if (getEnvBoolean('SPLUNK_PROFILER_ENABLED', false)) { - startProfiling(); + startProfiling(profilingOptions); } - startTracing(); + startTracing(tracingOptions); if (getEnvBoolean('SPLUNK_AUTOMATIC_LOG_COLLECTION', false)) { - startLogging(); + startLogging(loggingOptions); } if ( getEnvBoolean('SPLUNK_METRICS_ENABLED', false) || getEnvBoolean('SPLUNK_PROFILER_MEMORY_ENABLED', false) ) { - startMetrics(); + startMetrics(metricsOptions); } } diff --git a/src/instrumentations/graphql.ts b/src/instrumentations/graphql.ts index 26106ca0..104deb3c 100644 --- a/src/instrumentations/graphql.ts +++ b/src/instrumentations/graphql.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import { Options } from '../tracing/options'; +import { StartTracingOptions } from '../tracing'; import { getEnvBoolean } from '../utils'; import type { GraphQLInstrumentation, @@ -24,7 +24,7 @@ import type { export function configureGraphQlInstrumentation( // eslint-disable-next-line @typescript-eslint/no-explicit-any instrumentation: any, - _options: Options + _options: StartTracingOptions ) { if (getEnvBoolean('SPLUNK_GRAPHQL_RESOLVE_SPANS_ENABLED', false)) { return; diff --git a/src/instrumentations/http.ts b/src/instrumentations/http.ts index 5277a326..1a003fe5 100644 --- a/src/instrumentations/http.ts +++ b/src/instrumentations/http.ts @@ -14,7 +14,8 @@ * limitations under the License. */ -import { Options, CaptureHttpUriParameters } from '../tracing/options'; +import { CaptureHttpUriParameters } from '../tracing/options'; +import { Options as TracingOptions } from '../tracing/options'; import { IncomingMessage, ServerResponse } from 'http'; import { HttpInstrumentationConfig, @@ -27,7 +28,7 @@ import * as Url from 'url'; type IncomingHttpRequestHook = (span: Span, request: IncomingMessage) => void; -function shouldAddRequestHook(options: Options): boolean { +function shouldAddRequestHook(options: TracingOptions): boolean { if ( Array.isArray(options.captureHttpRequestUriParams) && options.captureHttpRequestUriParams.length === 0 @@ -86,7 +87,7 @@ function captureUriParamByFunction( } function createHttpRequestHook( - options: Options + options: TracingOptions ): HttpRequestCustomAttributeFunction { const incomingRequestHooks: IncomingHttpRequestHook[] = []; @@ -118,7 +119,7 @@ function createHttpRequestHook( export function configureHttpInstrumentation( // eslint-disable-next-line @typescript-eslint/no-explicit-any instrumentation: any, - options: Options + options: TracingOptions ) { if (!options.serverTimingEnabled) { return; diff --git a/src/instrumentations/index.ts b/src/instrumentations/index.ts index 886dee6c..accde421 100644 --- a/src/instrumentations/index.ts +++ b/src/instrumentations/index.ts @@ -15,8 +15,30 @@ */ import { load } from './loader'; -import { getEnvBoolean } from '../utils'; +import { getEnvBoolean, assertNoExtraneousProperties, pick } from '../utils'; import type { EnvVarKey } from '../types'; +import { + allowedTracingOptions, + Options as TracingOptions, + _setDefaultOptions as setDefaultTracingOptions, +} from '../tracing/options'; + +import type { StartLoggingOptions } from '../logging'; +import { + allowedLoggingOptions, + _setDefaultOptions as setDefaultLoggingOptions, +} from '../logging'; +import { allowedProfilingOptions } from '../profiling/types'; +import { _setDefaultOptions as setDefaultProfilingOptions } from '../profiling'; +import { + allowedMetricsOptions, + _setDefaultOptions as setDefaultMetricsOptions, +} from '../metrics'; +import type { Options as StartOptions } from '../start'; +import { configureGraphQlInstrumentation } from './graphql'; +import { configureHttpInstrumentation } from './http'; +import { configureLogInjection, disableLogSending } from './logging'; +import { configureRedisInstrumentation } from './redis'; type InstrumentationInfo = { module: string; @@ -24,6 +46,11 @@ type InstrumentationInfo = { shortName: string; }; +interface Options { + tracing: TracingOptions; + logging: StartLoggingOptions; +} + export const bundledInstrumentations: InstrumentationInfo[] = [ { module: '@opentelemetry/instrumentation-amqplib', @@ -233,3 +260,82 @@ export function getInstrumentations() { return loaded; } + +export function configureInstrumentations(options: Options) { + const instrumentations = options.tracing.instrumentations || []; + for (const instrumentation of instrumentations) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const instr = instrumentation as any; + + switch (instr['instrumentationName']) { + case '@opentelemetry/instrumentation-graphql': + configureGraphQlInstrumentation(instr, options.tracing); + break; + case '@opentelemetry/instrumentation-http': + configureHttpInstrumentation(instr, options.tracing); + break; + case '@opentelemetry/instrumentation-redis': + configureRedisInstrumentation(instr, options.tracing); + break; + case '@opentelemetry/instrumentation-bunyan': + disableLogSending(instr); + configureLogInjection(instr); + break; + case '@opentelemetry/instrumentation-pino': + case '@opentelemetry/instrumentation-winston': + configureLogInjection(instr); + break; + } + } +} + +export function parseOptionsAndConfigureInstrumentations( + options: Partial = {} +) { + const { metrics, profiling, tracing, logging, ...restOptions } = options; + + assertNoExtraneousProperties(restOptions, [ + 'accessToken', + 'endpoint', + 'serviceName', + 'logLevel', + ]); + + const startProfilingOptions = Object.assign( + pick(restOptions, allowedProfilingOptions), + profiling + ); + + assertNoExtraneousProperties(startProfilingOptions, allowedProfilingOptions); + const profilingOptions = setDefaultProfilingOptions(startProfilingOptions); + + const startLoggingOptions = Object.assign( + pick(restOptions, allowedLoggingOptions), + logging + ); + + const loggingOptions = setDefaultLoggingOptions(startLoggingOptions); + + const startTracingOptions = Object.assign( + pick(restOptions, allowedTracingOptions), + tracing + ) as Partial; + + assertNoExtraneousProperties(startTracingOptions, allowedTracingOptions); + const tracingOptions = setDefaultTracingOptions(startTracingOptions); + + const startMetricsOptions = Object.assign( + pick(restOptions, allowedMetricsOptions), + metrics + ); + + assertNoExtraneousProperties(startMetricsOptions, allowedMetricsOptions); + const metricsOptions = setDefaultMetricsOptions(startMetricsOptions); + + configureInstrumentations({ + tracing: tracingOptions, + logging: loggingOptions, + }); + + return { tracingOptions, loggingOptions, profilingOptions, metricsOptions }; +} diff --git a/src/instrumentations/redis.ts b/src/instrumentations/redis.ts index 854bd365..f304b834 100644 --- a/src/instrumentations/redis.ts +++ b/src/instrumentations/redis.ts @@ -19,12 +19,12 @@ import type { RedisInstrumentationConfig, } from '@opentelemetry/instrumentation-redis'; import { getEnvBoolean } from '../utils'; -import { Options } from '../tracing/options'; +import { StartTracingOptions } from '../tracing'; export function configureRedisInstrumentation( // eslint-disable-next-line @typescript-eslint/no-explicit-any instrumentation: any, - _options: Options + _options: StartTracingOptions ) { const redisInstrumentation = instrumentation as RedisInstrumentation; if (getEnvBoolean('SPLUNK_REDIS_INCLUDE_COMMAND_ARGS', false)) { diff --git a/src/logging/index.ts b/src/logging/index.ts index 8eda5ae1..971ad2de 100644 --- a/src/logging/index.ts +++ b/src/logging/index.ts @@ -31,10 +31,10 @@ import { getNonEmptyEnvVar, getEnvArray, defaultServiceName } from '../utils'; import { detect as detectResource } from '../resource'; type LogRecordProcessorFactory = ( - options: LoggingOptions + options: Options ) => LogRecordProcessor | LogRecordProcessor[]; -interface LoggingOptions { +export interface Options { accessToken?: string; realm?: string; serviceName: string; @@ -51,10 +51,9 @@ export const allowedLoggingOptions = [ 'logRecordProcessorFactory', ]; -export type StartLoggingOptions = Partial>; +export type StartLoggingOptions = Partial>; -export function startLogging(opts: StartLoggingOptions = {}) { - const options = _setDefaultOptions(opts); +export function startLogging(options: Options) { const loggerProvider = new LoggerProvider({ resource: options.resource, }); @@ -78,9 +77,7 @@ export function startLogging(opts: StartLoggingOptions = {}) { }; } -export function _setDefaultOptions( - options: StartLoggingOptions = {} -): LoggingOptions { +export function _setDefaultOptions(options: StartLoggingOptions = {}): Options { let resource = detectResource(); const serviceName = @@ -122,7 +119,7 @@ function areValidExporterTypes(types: string[]): boolean { return types.every((t) => SUPPORTED_EXPORTER_TYPES.includes(t)); } -function createExporters(options: LoggingOptions) { +function createExporters(options: Options) { const logExporters: string[] = getEnvArray('OTEL_LOGS_EXPORTER', ['otlp']); if (!areValidExporterTypes(logExporters)) { @@ -150,7 +147,7 @@ function createExporters(options: LoggingOptions) { } export function defaultlogRecordProcessorFactory( - options: LoggingOptions + options: Options ): LogRecordProcessor[] { let exporters = createExporters(options); diff --git a/src/metrics/index.ts b/src/metrics/index.ts index 6873a7bf..2b5faa6d 100644 --- a/src/metrics/index.ts +++ b/src/metrics/index.ts @@ -26,7 +26,6 @@ import { OTLPMetricExporter as OTLPHttpProtoMetricExporter } from '@opentelemetr import type * as grpc from '@grpc/grpc-js'; import type * as OtlpGrpc from '@opentelemetry/exporter-metrics-otlp-grpc'; import { - assertNoExtraneousProperties, defaultServiceName, getEnvArray, getEnvBoolean, @@ -43,7 +42,7 @@ import { ConsoleMetricExporter } from './ConsoleMetricExporter'; export type MetricReaderFactory = (options: MetricsOptions) => MetricReader[]; export type ResourceFactory = (resource: Resource) => Resource; -interface MetricsOptions { +export interface MetricsOptions { accessToken: string; realm?: string; serviceName: string; @@ -244,11 +243,7 @@ export const allowedMetricsOptions = [ 'debugMetricsEnabled', ]; -export function startMetrics(opts: StartMetricsOptions = {}) { - assertNoExtraneousProperties(opts, allowedMetricsOptions); - - const options = _setDefaultOptions(opts); - +export function startMetrics(options: MetricsOptions) { const debugMetricsViews: View[] = options.debugMetricsEnabled ? getDebugMetricsViews() : []; diff --git a/src/profiling/index.ts b/src/profiling/index.ts index acdf9952..5fd09142 100644 --- a/src/profiling/index.ts +++ b/src/profiling/index.ts @@ -17,7 +17,6 @@ import { context, diag } from '@opentelemetry/api'; import { Resource } from '@opentelemetry/resources'; import { - assertNoExtraneousProperties, defaultServiceName, getEnvBoolean, getEnvNumber, @@ -37,7 +36,6 @@ import { ProfilingOptions, StartProfilingOptions, ProfilingStartOptions, - allowedProfilingOptions, } from './types'; import { ProfilingContextManager } from './ProfilingContextManager'; import { OTLPProfilingExporter } from './OTLPProfilingExporter'; @@ -101,11 +99,7 @@ export function isProfilingContextManagerSet(): boolean { return profilingContextManagerEnabled; } -export function startProfiling(opts: StartProfilingOptions = {}) { - assertNoExtraneousProperties(opts, allowedProfilingOptions); - - const options = _setDefaultOptions(opts); - +export function startProfiling(options: ProfilingOptions) { const extension = loadExtension(); if (extension === undefined) { diff --git a/src/start.ts b/src/start.ts index 4507c8dd..feef7521 100644 --- a/src/start.ts +++ b/src/start.ts @@ -14,20 +14,14 @@ * limitations under the License. */ import { - assertNoExtraneousProperties, getEnvBoolean, getNonEmptyEnvVar, parseEnvBooleanString, parseLogLevel, - pick, toDiagLogLevel, } from './utils'; import { startMetrics, StartMetricsOptions } from './metrics'; -import { - startProfiling, - StartProfilingOptions, - _setDefaultOptions as setDefaultProfilingOptions, -} from './profiling'; +import { startProfiling, StartProfilingOptions } from './profiling'; import type { EnvVarKey, LogLevel } from './types'; import { getLoadedInstrumentations, @@ -35,9 +29,7 @@ import { stopTracing, StartTracingOptions, } from './tracing'; -import { allowedTracingOptions } from './tracing/options'; -import { allowedProfilingOptions } from './profiling/types'; -import { allowedMetricsOptions } from './metrics'; +import { parseOptionsAndConfigureInstrumentations } from './instrumentations'; import { diag, DiagConsoleLogger, @@ -46,13 +38,9 @@ import { MeterOptions, createNoopMeter, } from '@opentelemetry/api'; -import { - StartLoggingOptions, - allowedLoggingOptions, - startLogging, -} from './logging'; +import { StartLoggingOptions, startLogging } from './logging'; -interface Options { +export interface Options { accessToken: string; endpoint: string; serviceName: string; @@ -95,15 +83,6 @@ export const start = (options: Partial = {}) => { ) { throw new Error('Splunk APM already started'); } - const { metrics, profiling, tracing, logging, ...restOptions } = options; - - assertNoExtraneousProperties(restOptions, [ - 'accessToken', - 'endpoint', - 'serviceName', - 'logLevel', - ]); - const logLevel = options.logLevel ? toDiagLogLevel(options.logLevel) : parseLogLevel(getNonEmptyEnvVar('OTEL_LOG_LEVEL')); @@ -112,35 +91,25 @@ export const start = (options: Partial = {}) => { diag.setLogger(new DiagConsoleLogger(), logLevel); } + const { tracingOptions, loggingOptions, profilingOptions, metricsOptions } = + parseOptionsAndConfigureInstrumentations(options); + let metricsEnabledByDefault = false; if (isSignalEnabled(options.profiling, 'SPLUNK_PROFILER_ENABLED', false)) { - const profilingOptions = Object.assign( - pick(restOptions, allowedProfilingOptions), - profiling - ); running.profiling = startProfiling(profilingOptions); - - // HACK: memory profiling needs to enable metrics, - // run the default option function to see whether memory profiling is enabled - const materializedOptions = setDefaultProfilingOptions(profilingOptions); - - if (materializedOptions.memoryProfilingEnabled) { + if (profilingOptions.memoryProfilingEnabled) { metricsEnabledByDefault = true; } } if (isSignalEnabled(options.tracing, 'SPLUNK_TRACING_ENABLED', true)) { - running.tracing = startTracing( - Object.assign(pick(restOptions, allowedTracingOptions), tracing) - ); + running.tracing = startTracing(tracingOptions); } if ( isSignalEnabled(options.logging, 'SPLUNK_AUTOMATIC_LOG_COLLECTION', false) ) { - running.logging = startLogging( - Object.assign(pick(restOptions, allowedLoggingOptions), logging) - ); + running.logging = startLogging(loggingOptions); } if ( @@ -150,9 +119,7 @@ export const start = (options: Partial = {}) => { metricsEnabledByDefault ) ) { - running.metrics = startMetrics( - Object.assign(pick(restOptions, allowedMetricsOptions), metrics) - ); + running.metrics = startMetrics(metricsOptions); } const meterProvider = getEnvBoolean( diff --git a/src/tracing/index.ts b/src/tracing/index.ts index 3e5bf065..4e42752a 100644 --- a/src/tracing/index.ts +++ b/src/tracing/index.ts @@ -35,20 +35,8 @@ import { AsyncHooksContextManager, AsyncLocalStorageContextManager, } from '@opentelemetry/context-async-hooks'; - -import { configureGraphQlInstrumentation } from '../instrumentations/graphql'; -import { configureHttpInstrumentation } from '../instrumentations/http'; -import { - configureLogInjection, - disableLogSending, -} from '../instrumentations/logging'; -import { allowedTracingOptions, Options, _setDefaultOptions } from './options'; -import { configureRedisInstrumentation } from '../instrumentations/redis'; -import { - assertNoExtraneousProperties, - getNonEmptyEnvVar, - parseEnvBooleanString, -} from '../utils'; +import { Options } from './options'; +import { getNonEmptyEnvVar, parseEnvBooleanString } from '../utils'; import { isProfilingContextManagerSet } from '../profiling'; /** @@ -97,14 +85,10 @@ function setLoadedInstrumentations(instrumentations: InstrumentationOption[]) { } export type StartTracingOptions = Partial; -export function startTracing(opts: StartTracingOptions = {}): boolean { +export function startTracing(options: Options): boolean { assert(!isStarted, 'Splunk APM already started'); isStarted = true; - assertNoExtraneousProperties(opts, allowedTracingOptions); - - const options = _setDefaultOptions(opts); - // propagator propagation.setGlobalPropagator(options.propagatorFactory(options)); @@ -130,8 +114,6 @@ export function startTracing(opts: StartTracingOptions = {}): boolean { process.env.OTEL_TRACES_EXPORTER = envTracesExporter; } - configureInstrumentations(options); - // instrumentations unregisterInstrumentations = registerInstrumentations({ tracerProvider: provider, @@ -211,30 +193,3 @@ async function shutdownGlobalTracerProvider() { }) does not implement shutdown()` ); } - -function configureInstrumentations(options: Options) { - for (const instrumentation of options.instrumentations) { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const instr = instrumentation as any; - - switch (instr['instrumentationName']) { - case '@opentelemetry/instrumentation-graphql': - configureGraphQlInstrumentation(instr, options); - break; - case '@opentelemetry/instrumentation-http': - configureHttpInstrumentation(instr, options); - break; - case '@opentelemetry/instrumentation-redis': - configureRedisInstrumentation(instr, options); - break; - case '@opentelemetry/instrumentation-bunyan': - disableLogSending(instr); - configureLogInjection(instr); - break; - case '@opentelemetry/instrumentation-pino': - case '@opentelemetry/instrumentation-winston': - configureLogInjection(instr); - break; - } - } -} diff --git a/src/tracing/options.ts b/src/tracing/options.ts index 7994bfc8..e717ca82 100644 --- a/src/tracing/options.ts +++ b/src/tracing/options.ts @@ -31,9 +31,9 @@ import { detect as detectResource } from '../resource'; import { defaultServiceName, getEnvArray, - getEnvBoolean, getEnvValueByPrecedence, getNonEmptyEnvVar, + getEnvBoolean, } from '../utils'; import { NodeTracerConfig } from '@opentelemetry/sdk-trace-node'; import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; @@ -178,12 +178,12 @@ export function _setDefaultOptions(options: Partial = {}): Options { ), accessToken: options.accessToken, serverTimingEnabled: options.serverTimingEnabled, + captureHttpRequestUriParams: options.captureHttpRequestUriParams, instrumentations: options.instrumentations, tracerConfig: tracerConfig, spanExporterFactory: options.spanExporterFactory, spanProcessorFactory: options.spanProcessorFactory, propagatorFactory: options.propagatorFactory, - captureHttpRequestUriParams: options.captureHttpRequestUriParams, }; } diff --git a/test/api.test.ts b/test/api.test.ts index 2cd47e44..215b1f1a 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -21,6 +21,7 @@ import * as splunk from '../src'; import * as tracing from '../src/tracing'; import * as metrics from '../src/metrics'; import * as profiling from '../src/profiling'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; const SIGNALS = { tracing, @@ -65,8 +66,9 @@ describe('API', () => { }); it('should throw if start is called multiple times', () => { - api.startTracing(); - assert.throws(() => api.startTracing()); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + api.startTracing(tracingOptions); + assert.throws(() => api.startTracing(tracingOptions)); api.stopTracing(); }); diff --git a/test/instrumentation/redis.test.ts b/test/instrumentation/redis.test.ts index d1119b77..65175d9e 100644 --- a/test/instrumentation/redis.test.ts +++ b/test/instrumentation/redis.test.ts @@ -25,6 +25,7 @@ import * as utils from '../utils'; import * as net from 'net'; import type * as Redis from 'redis'; import { RedisInstrumentation } from '@opentelemetry/instrumentation-redis'; +import { parseOptionsAndConfigureInstrumentations } from '../../src/instrumentations'; describe('Redis instrumentation', () => { let redisServer; @@ -71,7 +72,10 @@ describe('Redis instrumentation', () => { it('db statement is not added when SPLUNK_REDIS_INCLUDE_COMMAND_ARGS is false', (done) => { process.env.SPLUNK_REDIS_INCLUDE_COMMAND_ARGS = 'false'; - startTracing(testOpts()); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: testOpts(), + }); + startTracing(tracingOptions); const client = require('redis').createClient({ no_ready_check: true, }); @@ -89,7 +93,10 @@ describe('Redis instrumentation', () => { it('db statement is fully added when setting SPLUNK_REDIS_INCLUDE_COMMAND_ARGS env var', (done) => { process.env.SPLUNK_REDIS_INCLUDE_COMMAND_ARGS = 'true'; - startTracing(testOpts()); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: testOpts(), + }); + startTracing(tracingOptions); const client = require('redis').createClient({ no_ready_check: true, }); diff --git a/test/logging.test.ts b/test/logging.test.ts index 7b7068ce..f6ccfe20 100644 --- a/test/logging.test.ts +++ b/test/logging.test.ts @@ -22,11 +22,13 @@ import { SimpleLogRecordProcessor, ConsoleLogRecordExporter, } from '@opentelemetry/sdk-logs'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; describe('logging', () => { describe('startLogging', () => { it('sets logprovider', () => { - startLogging(); + const { loggingOptions } = parseOptionsAndConfigureInstrumentations(); + startLogging(loggingOptions); const provider = logsAPI.logs.getLoggerProvider(); assert(provider instanceof LoggerProvider); }); diff --git a/test/loginjection.bunyan.test.ts b/test/loginjection.bunyan.test.ts index 3518d4a8..5c97c4fd 100644 --- a/test/loginjection.bunyan.test.ts +++ b/test/loginjection.bunyan.test.ts @@ -16,6 +16,7 @@ import { TestLogStream, assertInjection } from './utils'; import { startTracing, stopTracing } from '../src/tracing'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; import type * as bunyan from 'bunyan'; describe('log injection', () => { @@ -36,7 +37,10 @@ describe('log injection', () => { }); it('injects service version and service environment if available', () => { - startTracing({ serviceName: 'test-service' }); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { serviceName: 'test-service' }, + }); + startTracing(tracingOptions); const logger: bunyan = require('bunyan').createLogger({ name: 'test', diff --git a/test/loginjection.pino.test.ts b/test/loginjection.pino.test.ts index 8e5dccaf..6907d86d 100644 --- a/test/loginjection.pino.test.ts +++ b/test/loginjection.pino.test.ts @@ -19,7 +19,7 @@ import { startTracing, stopTracing } from '../src/tracing'; import { TestLogStream, assertInjection } from './utils'; import { defaultLogHook } from '../src/instrumentations/logging'; import { PinoInstrumentation } from '@opentelemetry/instrumentation-pino'; - +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; describe('pino with with custom hooks', () => { let logStream: TestLogStream; @@ -33,16 +33,19 @@ describe('pino with with custom hooks', () => { it('is possible to opt out from injecting resource attributes', () => { const MY_VALUE = 'myValue'; const MY_ATTRIBUTE = 'myAttribute'; - startTracing({ - serviceName: 'test-service', - instrumentations: [ - new PinoInstrumentation({ - logHook: (span, logRecord) => { - logRecord[MY_ATTRIBUTE] = MY_VALUE; - }, - }), - ], + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + serviceName: 'test-service', + instrumentations: [ + new PinoInstrumentation({ + logHook: (span, logRecord) => { + logRecord[MY_ATTRIBUTE] = MY_VALUE; + }, + }), + ], + }, }); + startTracing(tracingOptions); const logger: pino.Logger = require('pino')(logStream.stream); @@ -55,17 +58,20 @@ describe('pino with with custom hooks', () => { it('is easy enough do do both', () => { const MY_VALUE = 'myValueBoth'; const MY_ATTRIBUTE = 'myAttributeBoth'; - startTracing({ - serviceName: 'test-service', - instrumentations: [ - new PinoInstrumentation({ - logHook: (span, logRecord) => { - defaultLogHook(span, logRecord); - logRecord[MY_ATTRIBUTE] = MY_VALUE; - }, - }), - ], + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + serviceName: 'test-service', + instrumentations: [ + new PinoInstrumentation({ + logHook: (span, logRecord) => { + defaultLogHook(span, logRecord); + logRecord[MY_ATTRIBUTE] = MY_VALUE; + }, + }), + ], + }, }); + startTracing(tracingOptions); const logger: pino.Logger = require('pino')(logStream.stream); diff --git a/test/loginjection.winston.test.ts b/test/loginjection.winston.test.ts index e2e13910..59cabfab 100644 --- a/test/loginjection.winston.test.ts +++ b/test/loginjection.winston.test.ts @@ -16,6 +16,7 @@ import { startTracing, stopTracing } from '../src/tracing'; import { TestLogStream, assertInjection } from './utils'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; describe('winston log injection', () => { let logStream: TestLogStream; @@ -25,7 +26,10 @@ describe('winston log injection', () => { }); it('injects context to winston records', () => { - startTracing({ serviceName: 'test-service' }); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { serviceName: 'test-service' }, + }); + startTracing(tracingOptions); const winston = require('winston'); const logger = winston.createLogger({ transports: [new winston.transports.Stream({ stream: logStream.stream })], diff --git a/test/metrics.test.ts b/test/metrics.test.ts index 226cc973..0ea8941c 100644 --- a/test/metrics.test.ts +++ b/test/metrics.test.ts @@ -32,6 +32,7 @@ import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions' import { cleanEnvironment, TestMetricReader } from './utils'; import { hrtime } from 'process'; import { startMetrics, _setDefaultOptions } from '../src/metrics'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; function emptyCounter() { return { @@ -165,22 +166,25 @@ describe('metrics', () => { const resource = new Resource({ [SemanticResourceAttributes.DEPLOYMENT_ENVIRONMENT]: 'test', }); - - startMetrics({ - serviceName: 'foo', - resourceFactory: (defaultResource: Resource) => { - return defaultResource.merge(resource); - }, - views: [ - new View({ name: 'clicks.xyz', instrumentName: 'test-counter' }), - ], - runtimeMetricsEnabled: true, - runtimeMetricsCollectionIntervalMillis: 1, - metricReaderFactory: () => { - return [reader]; + const { metricsOptions } = parseOptionsAndConfigureInstrumentations({ + metrics: { + serviceName: 'foo', + resourceFactory: (defaultResource: Resource) => { + return defaultResource.merge(resource); + }, + views: [ + new View({ name: 'clicks.xyz', instrumentName: 'test-counter' }), + ], + runtimeMetricsEnabled: true, + runtimeMetricsCollectionIntervalMillis: 1, + metricReaderFactory: () => { + return [reader]; + }, }, }); + startMetrics(metricsOptions); + const counter = metrics.getMeter('custom').createCounter('test-counter'); counter.add(42); diff --git a/test/options.test.ts b/test/options.test.ts index 7c4aadfa..14f84a48 100644 --- a/test/options.test.ts +++ b/test/options.test.ts @@ -185,10 +185,6 @@ describe('options', () => { assert(exporter instanceof OTLPTraceExporter); sinon.assert.calledWithMatch(logger.warn, MATCH_SERVICE_NAME_WARNING); - sinon.assert.calledWithMatch( - logger.warn, - MATCH_NO_INSTRUMENTATIONS_WARNING - ); }); it('reads the container when setting default options', () => { @@ -227,7 +223,6 @@ describe('options', () => { spanExporterFactory: testSpanExporterFactory, spanProcessorFactory: testSpanProcessorFactory, propagatorFactory: testPropagatorFactory, - captureHttpRequestUriParams: ['timestamp'], }); assert.deepStrictEqual(options, { @@ -236,6 +231,7 @@ describe('options', () => { serviceName: 'custom-service-name', accessToken: 'custom-access-token', serverTimingEnabled: true, + captureHttpRequestUriParams: [], instrumentations: [testInstrumentation], tracerConfig: { resource: new Resource({ attr1: 'value' }), @@ -244,7 +240,6 @@ describe('options', () => { spanExporterFactory: testSpanExporterFactory, spanProcessorFactory: testSpanProcessorFactory, propagatorFactory: testPropagatorFactory, - captureHttpRequestUriParams: ['timestamp'], }); sinon.assert.neverCalledWithMatch(logger.warn, MATCH_SERVICE_NAME_WARNING); diff --git a/test/propagation.test.ts b/test/propagation.test.ts index bb09aa6b..af9b8918 100644 --- a/test/propagation.test.ts +++ b/test/propagation.test.ts @@ -33,6 +33,7 @@ import { import { SYNTHETIC_RUN_ID_FIELD } from '../src/tracing/SplunkBatchSpanProcessor'; import { defaultSpanProcessorFactory } from '../src/tracing/options'; import * as utils from './utils'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; function assertIncludes(arr: string[], item: string) { assert(Array.isArray(arr), `Expected an array got ${util.inspect(arr)}`); @@ -53,7 +54,8 @@ describe('propagation', () => { afterEach(stopTracing); it('must be set to w3c by default', () => { - startTracing(); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); assertIncludes(propagation.fields(), 'traceparent'); assertIncludes(propagation.fields(), 'tracestate'); assertIncludes(propagation.fields(), 'baggage'); @@ -84,7 +86,8 @@ describe('propagation', () => { it('has an option for b3multi', () => { process.env.OTEL_PROPAGATORS = 'b3multi'; - startTracing(); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); assertIncludes(propagation.fields(), 'x-b3-traceid'); assertIncludes(propagation.fields(), 'x-b3-spanid'); assertIncludes(propagation.fields(), 'x-b3-sampled'); @@ -110,7 +113,8 @@ describe('propagation', () => { it('has an option for b3', () => { process.env.OTEL_PROPAGATORS = 'b3'; - startTracing(); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); assertIncludes(propagation.fields(), 'b3'); const tracer = trace.getTracer('test-tracer'); @@ -131,7 +135,8 @@ describe('propagation', () => { }); it('must extract synthetic run id', () => { - startTracing(); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); assertIncludes(propagation.fields(), 'baggage'); const syntheticsTraceId = new RandomIdGenerator().generateTraceId(); @@ -152,13 +157,17 @@ describe('propagation', () => { it('must attach synthetic run id to exported spans', async () => { const exporter = new InMemorySpanExporter(); let spanProcessor: SpanProcessor; - startTracing({ - spanExporterFactory: () => exporter, - spanProcessorFactory: (options) => { - return ([spanProcessor] = defaultSpanProcessorFactory(options)); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + spanExporterFactory: () => exporter, + spanProcessorFactory: (options) => { + return ([spanProcessor] = defaultSpanProcessorFactory(options)); + }, }, }); + startTracing(tracingOptions); + assertIncludes(propagation.fields(), 'baggage'); const tracer = trace.getTracer('test-tracer'); diff --git a/test/servertiming.test.ts b/test/servertiming.test.ts index fe7c835d..9daed9e4 100644 --- a/test/servertiming.test.ts +++ b/test/servertiming.test.ts @@ -19,6 +19,7 @@ import { context, trace } from '@opentelemetry/api'; import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import { startTracing, stopTracing } from '../src/tracing'; import * as utils from './utils'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; const PORT = 9111; const SERVER_URL = `http://localhost:${PORT}`; @@ -59,45 +60,54 @@ describe('servertiming', () => { } it('injects server timing by default', (done) => { - startTracing({}); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); testHeadersAdded(done); }); it('can be enabled via environment variables', (done) => { process.env.SPLUNK_TRACE_RESPONSE_HEADER_ENABLED = 'true'; - startTracing({}); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); testHeadersAdded(() => { done(); }); }); it('injects server timing header with current context', (done) => { - startTracing({ serverTimingEnabled: true }); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { serverTimingEnabled: true }, + }); + startTracing(tracingOptions); testHeadersAdded(done); }); it('works with user provided http instrumentation config', (done) => { - startTracing({ - serverTimingEnabled: true, - instrumentations: [new HttpInstrumentation({})], + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + serverTimingEnabled: true, + instrumentations: [new HttpInstrumentation({})], + }, }); - + startTracing(tracingOptions); testHeadersAdded(done); }); it('leaves user hooks unchanged', (done) => { let userHookCalled = false; - - startTracing({ - serverTimingEnabled: true, - instrumentations: [ - new HttpInstrumentation({ - responseHook: (span, response) => { - userHookCalled = true; - }, - }), - ], + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + serverTimingEnabled: true, + instrumentations: [ + new HttpInstrumentation({ + responseHook: (span, response) => { + userHookCalled = true; + }, + }), + ], + }, }); + startTracing(tracingOptions); const http = require('http'); let spanContext; diff --git a/test/start.test.ts b/test/start.test.ts index 5cbd95b1..ba4aad17 100644 --- a/test/start.test.ts +++ b/test/start.test.ts @@ -20,6 +20,7 @@ import * as metrics from '../src/metrics'; import * as profiling from '../src/profiling'; import * as tracing from '../src/tracing'; import * as logging from '../src/logging'; +import { getInstrumentations } from '../src/instrumentations'; import { start, stop } from '../src'; import { Resource } from '@opentelemetry/resources'; import { diag, DiagConsoleLogger, DiagLogLevel } from '@opentelemetry/api'; @@ -37,9 +38,9 @@ CONFIG.general = { serviceName, }; -const exportInterval = 'exportInterval'; +const exportIntervalMillis = 'exportInterval'; CONFIG.metrics = { - exportInterval, + exportIntervalMillis, }; const callstackInterval = 'callstackInterval'; @@ -187,25 +188,27 @@ describe('start', () => { logging: true, }); - sinon.assert.calledOnceWithExactly(signals.start.tracing, { + sinon.assert.calledOnceWithMatch(signals.start.tracing, { accessToken: 'xyz', endpoint: 'localhost:1111', serviceName: 'test', + serverTimingEnabled: true, + captureHttpRequestUriParams: [], }); - sinon.assert.calledOnceWithExactly(signals.start.profiling, { + sinon.assert.calledOnceWithMatch(signals.start.profiling, { endpoint: 'localhost:1111', serviceName: 'test', }); - sinon.assert.calledOnceWithExactly(signals.start.metrics, { + sinon.assert.calledOnceWithMatch(signals.start.metrics, { accessToken: 'xyz', endpoint: 'localhost:1111', serviceName: 'test', }); - sinon.assert.calledOnceWithExactly(signals.start.logging, { - accessToken: 'xyz', + sinon.assert.calledOnceWithMatch(signals.start.logging, { + // accessToken: 'xyz', // FIXME logging doesn't use accessToken atm cause no ingest endpoint: 'localhost:1111', serviceName: 'test', }); @@ -234,7 +237,8 @@ describe('start', () => { start({ ...CONFIG.general, profiling: { - ...CONFIG.general, + endpoint: CONFIG.general.endpoint, + serviceName: CONFIG.general.serviceName, ...CONFIG.profiling, }, tracing: { diff --git a/test/tracing/tracing.test.ts b/test/tracing/tracing.test.ts index 05f27414..fda07d7b 100644 --- a/test/tracing/tracing.test.ts +++ b/test/tracing/tracing.test.ts @@ -27,6 +27,7 @@ import { import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'; import { OTLPTraceExporter } from '@opentelemetry/exporter-trace-otlp-grpc'; +import { parseOptionsAndConfigureInstrumentations } from '../../src/instrumentations'; import { startTracing, stopTracing } from '../../src/tracing'; import * as utils from '../utils'; @@ -70,7 +71,8 @@ describe('tracing:otlp', () => { } it('setups tracing with defaults', () => { - startTracing(); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); assertTracingPipeline('localhost:4317', 'unnamed-node-service', ''); stopTracing(); }); @@ -79,11 +81,15 @@ describe('tracing:otlp', () => { const endpoint = 'custom-endpoint:1111'; const serviceName = 'test-node-service'; const accessToken = '1234'; - startTracing({ - endpoint, - serviceName, - accessToken, + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + endpoint, + serviceName, + accessToken, + }, }); + + startTracing(tracingOptions); assertTracingPipeline(endpoint, serviceName, accessToken); stopTracing(); }); @@ -97,18 +103,23 @@ describe('tracing:otlp', () => { process.env.OTEL_SERVICE_NAME = serviceName; process.env.SPLUNK_ACCESS_TOKEN = accessToken; - startTracing(); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations(); + startTracing(tracingOptions); assertTracingPipeline(url, serviceName, accessToken); stopTracing(); }); it('sets up tracing with a single processor', () => { - startTracing({ - spanProcessorFactory: () => { - return new SimpleSpanProcessor(new ConsoleSpanExporter()); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + spanProcessorFactory: () => { + return new SimpleSpanProcessor(new ConsoleSpanExporter()); + }, }, }); + startTracing(tracingOptions); + sinon.assert.calledOnce(addSpanProcessorMock); const p1 = addSpanProcessorMock.getCall(0).args[0]; @@ -119,15 +130,19 @@ describe('tracing:otlp', () => { }); it('sets up tracing with multiple processors', () => { - startTracing({ - spanProcessorFactory: function (options) { - return [ - new SimpleSpanProcessor(new ConsoleSpanExporter()), - new BatchSpanProcessor(new InMemorySpanExporter()), - ]; + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + spanProcessorFactory: function (options) { + return [ + new SimpleSpanProcessor(new ConsoleSpanExporter()), + new BatchSpanProcessor(new InMemorySpanExporter()), + ]; + }, }, }); + startTracing(tracingOptions); + sinon.assert.calledTwice(addSpanProcessorMock); const p1 = addSpanProcessorMock.getCall(0).args[0]; @@ -156,10 +171,14 @@ describe('tracing:otlp', () => { const exportFn = sinon.spy(exporter, 'export'); const shutdownFn = sinon.spy(exporter, 'shutdown'); - startTracing({ - spanExporterFactory: () => exporter, + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + spanExporterFactory: () => exporter, + }, }); + startTracing(tracingOptions); + const storedTracer = trace.getTracer('test-tracer'); createSpan(); diff --git a/test/uri_parameter_capture.test.ts b/test/uri_parameter_capture.test.ts index 276d4fa6..7d0a2ffd 100644 --- a/test/uri_parameter_capture.test.ts +++ b/test/uri_parameter_capture.test.ts @@ -17,6 +17,7 @@ import * as assert from 'assert'; import { startTracing, stopTracing } from '../src/tracing'; import { defaultSpanProcessorFactory } from '../src/tracing/options'; +import { parseOptionsAndConfigureInstrumentations } from '../src/instrumentations'; import * as utils from './utils'; import { InMemorySpanExporter, @@ -70,7 +71,10 @@ describe('Capturing URI parameters', () => { }); it('no uri parameters are captured by default', async () => { - startTracing(testOpts()); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: testOpts(), + }); + startTracing(tracingOptions); setupServer(); const [span] = await doRequest(SERVER_URL); for (const key of Object.keys(span.attributes)) { @@ -79,10 +83,13 @@ describe('Capturing URI parameters', () => { }); it('uri parameters can be captured by keys', async () => { - startTracing({ - captureHttpRequestUriParams: ['sortBy', 'order', 'yes'], - ...testOpts(), + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + captureHttpRequestUriParams: ['sortBy', 'order', 'yes'], + ...testOpts(), + }, }); + startTracing(tracingOptions); setupServer(); const [span] = await doRequest( `${SERVER_URL}/foo?sortBy=name&order=asc&order=desc&quux=123&yes` @@ -99,19 +106,21 @@ describe('Capturing URI parameters', () => { }); it('uri parameters can be captured by user supplied function', async () => { - startTracing({ - captureHttpRequestUriParams: (span, params) => { - const value = params['order']; - if (value === undefined) { - return; - } - - const values = Array.isArray(value) ? value : [value]; - - span.setAttribute('http.request.param.order', values); + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + captureHttpRequestUriParams: (span, params) => { + const value = params['order']; + if (value === undefined) { + return; + } + const values = Array.isArray(value) ? value : [value]; + span.setAttribute('http.request.param.order', values); + }, + ...testOpts(), }, - ...testOpts(), }); + startTracing(tracingOptions); + setupServer(); const [span] = await doRequest( `${SERVER_URL}/foo?sortBy=name&order=asc&quux=123` @@ -124,10 +133,13 @@ describe('Capturing URI parameters', () => { }); it('uri parameter keys are normalized', async () => { - startTracing({ - captureHttpRequestUriParams: ["'();:@&=+$,/?#[]b ar._&-~-&123!*"], - ...testOpts(), + const { tracingOptions } = parseOptionsAndConfigureInstrumentations({ + tracing: { + captureHttpRequestUriParams: ["'();:@&=+$,/?#[]b ar._&-~-&123!*"], + ...testOpts(), + }, }); + startTracing(tracingOptions); setupServer(); const [span] = await doRequest( `${SERVER_URL}/foo?%27()%3B%3A%40%26%3D%2B%24%2C%2F%3F%23%5B%5Db%20ar._%26-~-%26123!*=42`