Skip to content

Commit

Permalink
Explicitly parse configuration variables by type (#1004)
Browse files Browse the repository at this point in the history
Previously, each configuration value is evaluated in order to parse it.
This patch removes that evaluation in favor of a more option-specific
conversion where string values are parsed as strings, lists as lists, and
so on.

---------

Co-authored-by: Noemi <[email protected]>
  • Loading branch information
jeffkreeftmeijer and unflxw authored Mar 21, 2024
1 parent b17140e commit e5fca24
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
bump: "patch"
type: "change"
---

Don’t evaluate environment variable values to read configuration

In previous versions of the Node.js integration, environment variables were evaluated to read their values. This version instead parses them based on their expected values.
59 changes: 58 additions & 1 deletion src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,12 @@ describe("Configuration", () => {
describe("with config in the environment", () => {
it("loads configuration from the environment", () => {
process.env["APPSIGNAL_ENABLE_STATSD"] = "true"
process.env["APPSIGNAL_ENABLE_HOST_METRICS"] = "false"
process.env["APPSIGNAL_DNS_SERVERS"] = "8.8.8.8,8.8.4.4"
const envOptions = {
enableStatsd: true
enableStatsd: true,
enableHostMetrics: false,
dnsServers: ["8.8.8.8", "8.8.4.4"]
}
const expectedConfig = {
...expectedDefaultConfig,
Expand All @@ -143,6 +147,59 @@ describe("Configuration", () => {
expect(config.sources.initial).toEqual({})
expect(config.sources.env).toEqual(envOptions)
})

describe("with a list as APPSIGNAL_DISABLE_DEFAULT_INSTRUMENTATIONS", () => {
it("sets disableDefaultInstrumentations to the list", () => {
process.env["APPSIGNAL_DISABLE_DEFAULT_INSTRUMENTATIONS"] =
"@opentelemetry/instrumentation-express,@opentelemetry/instrumentation-fastify,@opentelemetry/instrumentation-fs"
const envOptions = {
disableDefaultInstrumentations: [
"@opentelemetry/instrumentation-express",
"@opentelemetry/instrumentation-fastify",
"@opentelemetry/instrumentation-fs"
]
}
const expectedConfig = {
...expectedDefaultConfig,
...envOptions
}
config = new Configuration({})

expect(config.data).toEqual(expectedConfig)
})
})

describe("with APPSIGNAL_DISABLE_DEFAULT_INSTRUMENTATIONS set to true", () => {
it("sets disableDefaultInstrumentations to true", () => {
process.env["APPSIGNAL_DISABLE_DEFAULT_INSTRUMENTATIONS"] = "true"
const envOptions = {
disableDefaultInstrumentations: true
}
const expectedConfig = {
...expectedDefaultConfig,
...envOptions
}
config = new Configuration({})

expect(config.data).toEqual(expectedConfig)
})
})

describe("with APPSIGNAL_DISABLE_DEFAULT_INSTRUMENTATIONS set to false", () => {
it("sets disableDefaultInstrumentations to false", () => {
process.env["APPSIGNAL_DISABLE_DEFAULT_INSTRUMENTATIONS"] = "false"
const envOptions = {
disableDefaultInstrumentations: false
}
const expectedConfig = {
...expectedDefaultConfig,
...envOptions
}
config = new Configuration({})

expect(config.data).toEqual(expectedConfig)
})
})
})

describe("logFilePath", () => {
Expand Down
48 changes: 40 additions & 8 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@ import fs from "fs"
import { VERSION } from "./version"
import { isWritable } from "./utils"
import { AppsignalOptions } from "./config/options"
import { ENV_TO_KEY_MAPPING, PRIVATE_ENV_MAPPING } from "./config/configmap"
import {
ENV_TO_KEY_MAPPING,
PRIVATE_ENV_MAPPING,
BOOL_KEYS,
STRING_KEYS,
LIST_KEYS,
LIST_OR_BOOL_KEYS
} from "./config/configmap"

/**
* The AppSignal configuration object.
Expand Down Expand Up @@ -169,16 +176,41 @@ export class Configuration {
private _loadFromEnvironment(): { [key: string]: any } {
const conf: { [key: string]: any } = {}

Object.entries(ENV_TO_KEY_MAPPING).forEach(([k, v]) => {
STRING_KEYS.forEach(k => {
const current = process.env[k]

if (current) {
try {
// attempt to extract a value from a string
conf[v] = eval(current)
} catch (e) {
conf[v] = current
}
conf[ENV_TO_KEY_MAPPING[k]] = current
}
})

BOOL_KEYS.forEach(k => {
const current = process.env[k]

if (current == "true") {
conf[ENV_TO_KEY_MAPPING[k]] = true
} else if (current == "false") {
conf[ENV_TO_KEY_MAPPING[k]] = false
}
})

LIST_KEYS.forEach(k => {
const current = process.env[k]

if (current) {
conf[ENV_TO_KEY_MAPPING[k]] = current.split(",")
}
})

LIST_OR_BOOL_KEYS.forEach(k => {
const current = process.env[k]

if (current == "true") {
conf[ENV_TO_KEY_MAPPING[k]] = true
} else if (current == "false") {
conf[ENV_TO_KEY_MAPPING[k]] = false
} else if (current) {
conf[ENV_TO_KEY_MAPPING[k]] = current.split(",")
}
})

Expand Down
49 changes: 49 additions & 0 deletions src/config/configmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,52 @@ export const JS_TO_RUBY_MAPPING: Record<keyof AppsignalOptions, string> = {
statsdPort: "statsd_port",
workingDirectoryPath: "working_directory_path"
}

export const BOOL_KEYS: string[] = [
"APPSIGNAL_ACTIVE",
"APPSIGNAL_ENABLE_HOST_METRICS",
"APPSIGNAL_ENABLE_OPENTELEMETRY_HTTP",
"APPSIGNAL_ENABLE_MINUTELY_PROBES",
"APPSIGNAL_ENABLE_STATSD",
"APPSIGNAL_ENABLE_NGINX_METRICS",
"APPSIGNAL_FILES_WORLD_ACCESSIBLE",
"APPSIGNAL_RUNNING_IN_CONTAINER",
"APPSIGNAL_SEND_ENVIRONMENT_METADATA",
"APPSIGNAL_SEND_PARAMS",
"APPSIGNAL_SEND_SESSION_DATA"
]

export const STRING_KEYS: string[] = [
"APPSIGNAL_APP_ENV",
"APPSIGNAL_APP_NAME",
"APPSIGNAL_BIND_ADDRESS",
"APPSIGNAL_CA_FILE_PATH",
"APPSIGNAL_HOSTNAME",
"APPSIGNAL_HOST_ROLE",
"APPSIGNAL_HTTP_PROXY",
"APPSIGNAL_INITIALIZE_OPENTELEMETRY_SDK",
"APPSIGNAL_LOG",
"APPSIGNAL_LOG_LEVEL",
"APPSIGNAL_LOG_PATH",
"APPSIGNAL_LOGGING_ENDPOINT",
"APPSIGNAL_OPENTELEMETRY_PORT",
"APPSIGNAL_PUSH_API_ENDPOINT",
"APPSIGNAL_PUSH_API_KEY",
"APPSIGNAL_STATSD_PORT",
"APPSIGNAL_WORKING_DIRECTORY_PATH",
"APP_REVISION"
]

export const LIST_KEYS: string[] = [
"APPSIGNAL_DNS_SERVERS",
"APPSIGNAL_FILTER_PARAMETERS",
"APPSIGNAL_FILTER_SESSION_DATA",
"APPSIGNAL_IGNORE_ACTIONS",
"APPSIGNAL_IGNORE_ERRORS",
"APPSIGNAL_IGNORE_NAMESPACES",
"APPSIGNAL_REQUEST_HEADERS"
]

export const LIST_OR_BOOL_KEYS: string[] = [
"APPSIGNAL_DISABLE_DEFAULT_INSTRUMENTATIONS"
]

0 comments on commit e5fca24

Please sign in to comment.