elastic / elastic/apm-agent-nodejs

chore: proper fallback value for boolean "bogus" options

Open
#3,765 0 comments 0 reactions 1 assignee Claimed by @david-luna View on GitHub
agent-nodejs
Dominant language
JavaScript
Stars
594
Forks
244
Avg merge
1d 8h
Merged PRs (30d)
16

Description

# Description

When refactoring our configuration tests to run them in separate processes (#3762) an unwanted behaviour was discovered when a "bogus" boolean value is set in the configuration. This is when the value is not any of `true`, `false`, `"true"` or `"false"`.

Regarding bogus values the spec says
```
If an invalid value for a configuration option is provided (for example: breakdown_metrics="yes" or apiRequestTime="1h") then the agent MUST ignore the value (falling back to a config source with lower precedence) and SHOULD emit a log warning about the ignored value.
```

The node agent logs the warning but instead of falling back to a config source with lower precedence the value gets resolved to `undefined` ignoring sources of lower precedence like the defaults. As an example this behaviour with defaults would be okay for the options with `false` value as default (contextPropagationOnly, disableSend, errorOnAbortedRequests, opentelemetryBridgeEnabled, useElasticTraceparentHeader, usePathAsTransactionName) but not for options with dafault value set to `true` (active, breakdownMetrics, captureExceptions, captureHeaders, centralConfig, instrument, instrumentIncomingHTTPRequests, spanCompressionEnabled, verifyServerCert).

As a example we have this test that sets a bogus value for `ELASTIC_APM_ACTIVE` env var. this value wins over the other sources and when resolved we get `undefined` resulting in the agent being inactive.
https://github.com/elastic/apm-agent-nodejs/blob/0e3e002efbba39311e6c0e3c55192e3eb7fdf4f2/test/config/config.test.js#L345

# Possible approach

The agent should keep reference of the options provided from all sources instead of squashing them into a single object. then the normalization process should be aware of these sources to do the proper fallback if one option has a bogus value.

NOTE: as discussed internally this will be actioned when the test refactoring is done

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.