Skip to content

Commit

Permalink
fix: handle empty env vars as undefined (#693)
Browse files Browse the repository at this point in the history
  • Loading branch information
seemk authored Mar 14, 2023
1 parent 25bf04f commit db8ef52
Show file tree
Hide file tree
Showing 11 changed files with 148 additions and 35 deletions.
5 changes: 3 additions & 2 deletions src/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
defaultServiceName,
getEnvArray,
getEnvBoolean,
getNonEmptyEnvVar,
parseLogLevel,
} from './utils';

Expand All @@ -27,13 +28,13 @@ import { startProfiling } from './profiling';
import { startTracing } from './tracing';

function boot() {
const logLevel = parseLogLevel(process.env.OTEL_LOG_LEVEL);
const logLevel = parseLogLevel(getNonEmptyEnvVar('OTEL_LOG_LEVEL'));

if (logLevel !== DiagLogLevel.NONE) {
diag.setLogger(new DiagConsoleLogger(), logLevel);
}

if (process.env.SPLUNK_AUTOINSTRUMENT_PACKAGE_NAMES !== undefined) {
if (getNonEmptyEnvVar('SPLUNK_AUTOINSTRUMENT_PACKAGE_NAMES') !== undefined) {
const limitToPackages = getEnvArray(
'SPLUNK_AUTOINSTRUMENT_PACKAGE_NAMES',
[]
Expand Down
10 changes: 6 additions & 4 deletions src/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
getEnvBoolean,
getEnvNumber,
getEnvValueByPrecedence,
getNonEmptyEnvVar,
} from '../utils';
import * as util from 'util';
import { detect as detectResource } from '../resource';
Expand Down Expand Up @@ -195,7 +196,7 @@ function createExporters(options: MetricsOptions) {
if (!areValidExporterTypes(metricExporters)) {
throw new Error(
`Invalid value for OTEL_METRICS_EXPORTER env variable: ${util.inspect(
process.env.OTEL_METRICS_EXPORTER
getNonEmptyEnvVar('OTEL_METRICS_EXPORTER')
)}. Choose from ${util.inspect(SUPPORTED_EXPORTER_TYPES, {
compact: true,
})} or leave undefined.`
Expand Down Expand Up @@ -373,11 +374,12 @@ export function _setDefaultOptions(
options: StartMetricsOptions = {}
): MetricsOptions {
const accessToken =
options.accessToken || process.env.SPLUNK_ACCESS_TOKEN || '';
options.accessToken || getNonEmptyEnvVar('SPLUNK_ACCESS_TOKEN') || '';

const endpoint = options.endpoint || process.env.SPLUNK_METRICS_ENDPOINT;
const endpoint =
options.endpoint || getNonEmptyEnvVar('SPLUNK_METRICS_ENDPOINT');

const realm = options.realm || process.env.SPLUNK_REALM || '';
const realm = options.realm || getNonEmptyEnvVar('SPLUNK_REALM') || '';

if (realm) {
if (!accessToken) {
Expand Down
7 changes: 4 additions & 3 deletions src/profiling/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
defaultServiceName,
getEnvBoolean,
getEnvNumber,
getNonEmptyEnvVar,
} from '../utils';
import { detect as detectResource } from '../resource';
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
Expand Down Expand Up @@ -196,15 +197,15 @@ export function _setDefaultOptions(
): ProfilingOptions {
const endpoint =
options.endpoint ||
process.env.SPLUNK_PROFILER_LOGS_ENDPOINT ||
process.env.OTEL_EXPORTER_OTLP_ENDPOINT ||
getNonEmptyEnvVar('SPLUNK_PROFILER_LOGS_ENDPOINT') ||
getNonEmptyEnvVar('OTEL_EXPORTER_OTLP_ENDPOINT') ||
'http://localhost:4317';

const combinedResource = detectResource();

const serviceName = String(
options.serviceName ||
process.env.OTEL_SERVICE_NAME ||
getNonEmptyEnvVar('OTEL_SERVICE_NAME') ||
combinedResource.attributes[SemanticResourceAttributes.SERVICE_NAME] ||
defaultServiceName()
);
Expand Down
7 changes: 4 additions & 3 deletions src/profiling/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { gzip } from 'zlib';
import { promisify } from 'util';
import * as grpc from '@grpc/grpc-js';
import { diag } from '@opentelemetry/api';
import { getNonEmptyEnvVar } from '../utils';

import { perftools } from './proto/profile';
import type { HeapProfile, RawProfilingData } from './types';
Expand Down Expand Up @@ -251,9 +252,9 @@ export function parseEndpoint(endpoint: string): {
if (endpoint.startsWith('https://')) {
host = endpoint.substr(8);
credentials = grpc.credentials.createSsl(
maybeReadPath(process.env.OTEL_EXPORTER_OTLP_CERTIFICATE),
maybeReadPath(process.env.OTEL_EXPORTER_OTLP_CLIENT_KEY),
maybeReadPath(process.env.OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE)
maybeReadPath(getNonEmptyEnvVar('OTEL_EXPORTER_OTLP_CERTIFICATE')),
maybeReadPath(getNonEmptyEnvVar('OTEL_EXPORTER_OTLP_CLIENT_KEY')),
maybeReadPath(getNonEmptyEnvVar('OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE'))
);
} else if (endpoint.startsWith('http://')) {
host = endpoint.substr(7);
Expand Down
9 changes: 5 additions & 4 deletions src/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
import {
assertNoExtraneousProperties,
getNonEmptyEnvVar,
parseEnvBooleanString,
parseLogLevel,
toDiagLogLevel,
Expand All @@ -25,7 +26,7 @@ import {
StartProfilingOptions,
_setDefaultOptions as setDefaultProfilingOptions,
} from './profiling';
import type { LogLevel } from './types';
import type { EnvVarKey, LogLevel } from './types';
import { startTracing, stopTracing, StartTracingOptions } from './tracing';
import { diag, DiagConsoleLogger, DiagLogLevel } from '@opentelemetry/api';

Expand Down Expand Up @@ -54,10 +55,10 @@ const running: RunningState = {

function isSignalEnabled<T>(
option: T | undefined | null,
envVar: string,
envVar: EnvVarKey,
def: boolean
) {
return option ?? parseEnvBooleanString(process.env[envVar]) ?? def;
return option ?? parseEnvBooleanString(getNonEmptyEnvVar(envVar)) ?? def;
}

export const start = (options: Partial<Options> = {}) => {
Expand All @@ -75,7 +76,7 @@ export const start = (options: Partial<Options> = {}) => {

const logLevel = options.logLevel
? toDiagLogLevel(options.logLevel)
: parseLogLevel(process.env.OTEL_LOG_LEVEL);
: parseLogLevel(getNonEmptyEnvVar('OTEL_LOG_LEVEL'));

if (logLevel !== DiagLogLevel.NONE) {
diag.setLogger(new DiagConsoleLogger(), logLevel);
Expand Down
8 changes: 6 additions & 2 deletions src/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ import { configureHttpInstrumentation } from '../instrumentations/http';
import { configureLogInjection } from '../instrumentations/logging';
import { allowedTracingOptions, Options, _setDefaultOptions } from './options';
import { configureRedisInstrumentation } from '../instrumentations/redis';
import { assertNoExtraneousProperties, parseEnvBooleanString } from '../utils';
import {
assertNoExtraneousProperties,
getNonEmptyEnvVar,
parseEnvBooleanString,
} from '../utils';
import { isProfilingContextManagerSet } from '../profiling';

/**
Expand All @@ -48,7 +52,7 @@ import { isProfilingContextManagerSet } from '../profiling';
* leaks and is inperfect in terms of the end result.
*/
const allowDoubleStart = parseEnvBooleanString(
process.env.TEST_ALLOW_DOUBLE_START
getNonEmptyEnvVar('TEST_ALLOW_DOUBLE_START')
);
let isStarted = false;
let tracingContextManagerEnabled = false;
Expand Down
13 changes: 7 additions & 6 deletions src/tracing/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
getEnvArray,
getEnvBoolean,
getEnvValueByPrecedence,
getNonEmptyEnvVar,
} from '../utils';
import { NodeTracerConfig } from '@opentelemetry/sdk-trace-node';
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
Expand Down Expand Up @@ -89,14 +90,14 @@ export const allowedTracingOptions = [

export function _setDefaultOptions(options: Partial<Options> = {}): Options {
process.env.OTEL_SPAN_LINK_COUNT_LIMIT =
process.env.OTEL_SPAN_LINK_COUNT_LIMIT ?? '1000';
getNonEmptyEnvVar('OTEL_SPAN_LINK_COUNT_LIMIT') ?? '1000';
process.env.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT =
process.env.OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT ?? '12000';
getNonEmptyEnvVar('OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT') ?? '12000';

options.accessToken =
options.accessToken || process.env.SPLUNK_ACCESS_TOKEN || '';
options.accessToken || getNonEmptyEnvVar('SPLUNK_ACCESS_TOKEN') || '';

options.realm = options.realm || process.env.SPLUNK_REALM;
options.realm = options.realm || getNonEmptyEnvVar('SPLUNK_REALM');

if (options.realm) {
if (!options.accessToken) {
Expand All @@ -119,7 +120,7 @@ export function _setDefaultOptions(options: Partial<Options> = {}): Options {

const serviceName =
options.serviceName ||
process.env.OTEL_SERVICE_NAME ||
getNonEmptyEnvVar('OTEL_SERVICE_NAME') ||
resource.attributes[SemanticResourceAttributes.SERVICE_NAME];

if (!serviceName) {
Expand Down Expand Up @@ -313,7 +314,7 @@ export function consoleSpanExporterFactory(): SpanExporter {
// Temporary workaround until https://github.com/open-telemetry/opentelemetry-js/issues/3094 is resolved
function getBatchSpanProcessorConfig() {
// OTel uses its own parsed environment, we can just use the default env if the BSP delay is unset.
if (process.env.OTEL_BSP_SCHEDULE_DELAY !== undefined) {
if (getNonEmptyEnvVar('OTEL_BSP_SCHEDULE_DELAY') !== undefined) {
return undefined;
}

Expand Down
34 changes: 34 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,37 @@
*/

export type LogLevel = 'none' | 'verbose' | 'debug' | 'info' | 'warn' | 'error';

export type EnvVarKey =
| 'OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT'
| 'OTEL_BSP_SCHEDULE_DELAY'
| 'OTEL_EXPORTER_OTLP_CERTIFICATE'
| 'OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE'
| 'OTEL_EXPORTER_OTLP_CLIENT_KEY'
| 'OTEL_EXPORTER_OTLP_ENDPOINT'
| 'OTEL_EXPORTER_OTLP_METRICS_PROTOCOL'
| 'OTEL_EXPORTER_OTLP_PROTOCOL'
| 'OTEL_EXPORTER_OTLP_TRACES_ENDPOINT'
| 'OTEL_EXPORTER_OTLP_TRACES_PROTOCOL'
| 'OTEL_LOG_LEVEL'
| 'OTEL_METRIC_EXPORT_INTERVAL'
| 'OTEL_METRICS_EXPORTER'
| 'OTEL_PROPAGATORS'
| 'OTEL_SERVICE_NAME'
| 'OTEL_SPAN_LINK_COUNT_LIMIT'
| 'OTEL_TRACES_EXPORTER'
| 'SPLUNK_ACCESS_TOKEN'
| 'SPLUNK_AUTOINSTRUMENT_PACKAGE_NAMES'
| 'SPLUNK_METRICS_ENABLED'
| 'SPLUNK_METRICS_ENDPOINT'
| 'SPLUNK_PROFILER_CALL_STACK_INTERVAL'
| 'SPLUNK_PROFILER_ENABLED'
| 'SPLUNK_PROFILER_LOGS_ENDPOINT'
| 'SPLUNK_PROFILER_MEMORY_ENABLED'
| 'SPLUNK_REALM'
| 'SPLUNK_REDIS_INCLUDE_COMMAND_ARGS'
| 'SPLUNK_RUNTIME_METRICS_COLLECTION_INTERVAL'
| 'SPLUNK_RUNTIME_METRICS_ENABLED'
| 'SPLUNK_TRACE_RESPONSE_HEADER_ENABLED'
| 'SPLUNK_TRACING_ENABLED'
| 'TEST_ALLOW_DOUBLE_START';
36 changes: 27 additions & 9 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import { strict as assert } from 'assert';
import { diag, DiagLogLevel } from '@opentelemetry/api';
import type { LogLevel } from './types';
import type { EnvVarKey, LogLevel } from './types';
import { resolve } from 'path';
import * as fs from 'fs';

Expand Down Expand Up @@ -54,6 +54,24 @@ export function defaultServiceName(cache: ConfigCache = configCache): string {
return findServiceName(cache) || 'unnamed-node-service';
}

export function getNonEmptyEnvVar(key: EnvVarKey): string | undefined {
const value = process.env[key];

if (value !== undefined) {
const trimmed = value.trim();
if (trimmed.length === 0) {
diag.warn(
`Defined, but empty environment variable: '${key}'. The value will be considered as undefined.`
);
return undefined;
}

return trimmed;
}

return value;
}

export function parseEnvBooleanString(value?: string) {
if (typeof value !== 'string') {
return value;
Expand All @@ -72,8 +90,8 @@ export function parseEnvBooleanString(value?: string) {
throw new Error(`Invalid string representing boolean: ${value}`);
}

export function getEnvBoolean(key: string, defaultValue = true) {
const value = process.env[key];
export function getEnvBoolean(key: EnvVarKey, defaultValue = true) {
const value = getNonEmptyEnvVar(key);

if (value === undefined) {
return defaultValue;
Expand All @@ -86,8 +104,8 @@ export function getEnvBoolean(key: string, defaultValue = true) {
return true;
}

export function getEnvNumber(key: string, defaultValue: number): number {
const value = process.env[key];
export function getEnvNumber(key: EnvVarKey, defaultValue: number): number {
const value = getNonEmptyEnvVar(key);

if (value === undefined) {
return defaultValue;
Expand All @@ -106,8 +124,8 @@ export function deduplicate(arr: string[]) {
return [...new Set(arr)];
}

export function getEnvArray(key: string, defaultValue: string[]): string[] {
const value = process.env[key];
export function getEnvArray(key: EnvVarKey, defaultValue: string[]): string[] {
const value = getNonEmptyEnvVar(key);

if (value === undefined) {
return defaultValue;
Expand All @@ -117,11 +135,11 @@ export function getEnvArray(key: string, defaultValue: string[]): string[] {
}

export function getEnvValueByPrecedence(
keys: string[],
keys: EnvVarKey[],
defaultValue?: string
): string | undefined {
for (const key of keys) {
const value = process.env[key];
const value = getNonEmptyEnvVar(key);

if (value !== undefined) {
return value;
Expand Down
2 changes: 1 addition & 1 deletion test/examples/collector.config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ service:
pipelines:
traces:
receivers: [otlp]
exporters: [httpsink]
exporters: [httpsink, logging/debug]
logs/profiling:
receivers: [otlp]
exporters: [logging/debug]
Loading

0 comments on commit db8ef52

Please sign in to comment.