Allow APM env to be set via the config file (#149266)

Part of #140818.

Kibana can read APM configuration from its own config or environment
variables. For the `environment` value, it can also use the `NODE_ENV`
value. However, `NODE_ENV` is often set, and due to the way the
configuration is merged, the env vars always override the config file.
In the case of the `environment`, this makes it easy to accidentally
clobber any `environment` that you set in the config file.

Tweak the config loading so that `NODE_ENV` is only used if the
`environment` isn't set in the config.
This commit is contained in:
Rory Hunter 2023-01-20 10:02:11 +00:00 committed by GitHub
parent 6051977fbd
commit f90a89a839
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 54 additions and 20 deletions

View file

@ -156,27 +156,52 @@ describe('ApmConfiguration', () => {
delete process.env.NODE_ENV;
});
it('correctly sets environment by reading env vars', () => {
let config = new ApmConfiguration(mockedRootDir, {}, false);
expect(config.getConfig('serviceName')).toEqual(
expect.objectContaining({
environment: 'development',
})
);
describe('correctly sets environment by reading env vars', () => {
it('no env var', () => {
const config = new ApmConfiguration(mockedRootDir, {}, false);
expect(config.getConfig('serviceName')).toEqual(
expect.objectContaining({
environment: 'development',
})
);
});
it('NODE_ENV', () => {
process.env.NODE_ENV = 'production';
const config = new ApmConfiguration(mockedRootDir, {}, false);
expect(config.getConfig('serviceName')).toEqual(
expect.objectContaining({
environment: 'production',
})
);
});
it('ELASTIC_APM_ENVIRONMENT', () => {
process.env.ELASTIC_APM_ENVIRONMENT = 'ci';
const config = new ApmConfiguration(mockedRootDir, {}, false);
expect(config.getConfig('serviceName')).toEqual(
expect.objectContaining({
environment: 'ci',
})
);
});
});
it('does not override the environment from NODE_ENV if already set in the config file', () => {
const kibanaConfig = {
elastic: {
apm: {
environment: 'local',
},
},
};
process.env.NODE_ENV = 'production';
config = new ApmConfiguration(mockedRootDir, {}, false);
expect(config.getConfig('serviceName')).toEqual(
expect.objectContaining({
environment: 'production',
})
);
process.env.ELASTIC_APM_ENVIRONMENT = 'ci';
config = new ApmConfiguration(mockedRootDir, {}, false);
const config = new ApmConfiguration(mockedRootDir, kibanaConfig, false);
expect(config.getConfig('serviceName')).toEqual(
expect.objectContaining({
environment: 'ci',
environment: 'local',
})
);
});

View file

@ -111,7 +111,7 @@ export class ApmConfiguration {
/**
* Override some config values when specific environment variables are used
*/
private getConfigFromEnv(): AgentConfigOptions {
private getConfigFromEnv(configFromKibanaConfig: AgentConfigOptions): AgentConfigOptions {
const config: AgentConfigOptions = {};
if (process.env.ELASTIC_APM_ACTIVE === 'true') {
@ -124,8 +124,15 @@ export class ApmConfiguration {
config.contextPropagationOnly = false;
}
if (process.env.ELASTIC_APM_ENVIRONMENT || process.env.NODE_ENV) {
config.environment = process.env.ELASTIC_APM_ENVIRONMENT || process.env.NODE_ENV;
if (process.env.ELASTIC_APM_ENVIRONMENT) {
config.environment = process.env.ELASTIC_APM_ENVIRONMENT;
} else {
// We check NODE_ENV in a different way so that, unlike
// ELASTIC_APM_ENVIRONMENT, it does not override any explicit value set
// in the config file.
if (!configFromKibanaConfig.environment && process.env.NODE_ENV) {
config.environment = process.env.NODE_ENV;
}
}
if (process.env.ELASTIC_APM_TRANSACTION_SAMPLE_RATE) {
@ -256,7 +263,9 @@ export class ApmConfiguration {
* Reads APM configuration from different sources and merges them together.
*/
private getConfigFromAllSources(): AgentConfigOptions {
const config = merge({}, this.getConfigFromKibanaConfig(), this.getConfigFromEnv());
const configFromKibanaConfig = this.getConfigFromKibanaConfig();
const configFromEnv = this.getConfigFromEnv(configFromKibanaConfig);
const config = merge({}, configFromKibanaConfig, configFromEnv);
if (config.active === false && config.contextPropagationOnly !== false) {
throw new Error(