Skip to content

Commit

Permalink
Move src/apm.js to @kbn/apm-config-loader (#94315)
Browse files Browse the repository at this point in the history
* Move src/apm.js to @kbn/apm-config-loader

* add unit tests for `initApm`

* return undefined instead of empty config

* use ?.
  • Loading branch information
pgayvallet authored Mar 14, 2021
1 parent 0c3514c commit ed5bbda
Show file tree
Hide file tree
Showing 12 changed files with 169 additions and 69 deletions.
21 changes: 19 additions & 2 deletions packages/kbn-apm-config-loader/src/config_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@

import { getConfigurationFilePaths, getConfigFromFiles, applyConfigOverrides } from './utils';
import { ApmConfiguration } from './config';
import { ApmAgentConfig } from './types';

let apmConfig: ApmConfiguration | undefined;

/**
* Load the APM configuration.
*
* @param argv the `process.argv` arguments
* @param rootDir The root directory of kibana (where the sources and the `package.json` file are)
* @param production true for production builds, false otherwise
* @param isDistributable true for production builds, false otherwise
*/
export const loadConfiguration = (
argv: string[],
Expand All @@ -24,5 +27,19 @@ export const loadConfiguration = (
const configPaths = getConfigurationFilePaths(argv);
const rawConfiguration = getConfigFromFiles(configPaths);
applyConfigOverrides(rawConfiguration, argv);
return new ApmConfiguration(rootDir, rawConfiguration, isDistributable);

apmConfig = new ApmConfiguration(rootDir, rawConfiguration, isDistributable);
return apmConfig;
};

export const getConfiguration = (serviceName: string): ApmAgentConfig | undefined => {
// integration test runner starts a kibana server that import the module without initializing APM.
// so we need to check initialization of the config.
// note that we can't just load the configuration during this module's import
// because jest IT are ran with `--config path-to-jest-config.js` which conflicts with the CLI's `config` arg
// causing the config loader to try to load the jest js config as yaml and throws.
if (apmConfig) {
return apmConfig.getConfig(serviceName);
}
return undefined;
};
3 changes: 2 additions & 1 deletion packages/kbn-apm-config-loader/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

export { loadConfiguration } from './config_loader';
export { getConfiguration } from './config_loader';
export { initApm } from './init_apm';
export type { ApmConfiguration } from './config';
export type { ApmAgentConfig } from './types';
12 changes: 12 additions & 0 deletions packages/kbn-apm-config-loader/src/init_apm.test.mocks.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export const mockLoadConfiguration = jest.fn();
jest.doMock('./config_loader', () => ({
loadConfiguration: mockLoadConfiguration,
}));
66 changes: 66 additions & 0 deletions packages/kbn-apm-config-loader/src/init_apm.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { mockLoadConfiguration } from './init_apm.test.mocks';

import { initApm } from './init_apm';
import apm from 'elastic-apm-node';

describe('initApm', () => {
let apmAddFilterSpy: jest.SpyInstance;
let apmStartSpy: jest.SpyInstance;
let getConfig: jest.Mock;

beforeEach(() => {
apmAddFilterSpy = jest.spyOn(apm, 'addFilter').mockImplementation(() => undefined);
apmStartSpy = jest.spyOn(apm, 'start').mockImplementation(() => undefined as any);
getConfig = jest.fn();

mockLoadConfiguration.mockImplementation(() => ({
getConfig,
}));
});

afterEach(() => {
jest.restoreAllMocks();
mockLoadConfiguration.mockReset();
});

it('calls `loadConfiguration` with the correct options', () => {
initApm(['foo', 'bar'], 'rootDir', true, 'service-name');

expect(mockLoadConfiguration).toHaveBeenCalledTimes(1);
expect(mockLoadConfiguration).toHaveBeenCalledWith(['foo', 'bar'], 'rootDir', true);
});

it('calls `apmConfigLoader.getConfig` with the correct options', () => {
initApm(['foo', 'bar'], 'rootDir', true, 'service-name');

expect(getConfig).toHaveBeenCalledTimes(1);
expect(getConfig).toHaveBeenCalledWith('service-name');
});

it('registers a filter using `addFilter`', () => {
initApm(['foo', 'bar'], 'rootDir', true, 'service-name');

expect(apmAddFilterSpy).toHaveBeenCalledTimes(1);
expect(apmAddFilterSpy).toHaveBeenCalledWith(expect.any(Function));
});

it('starts apm with the config returned from `getConfig`', () => {
const config = {
foo: 'bar',
};
getConfig.mockReturnValue(config);

initApm(['foo', 'bar'], 'rootDir', true, 'service-name');

expect(apmStartSpy).toHaveBeenCalledTimes(1);
expect(apmStartSpy).toHaveBeenCalledWith(config);
});
});
39 changes: 39 additions & 0 deletions packages/kbn-apm-config-loader/src/init_apm.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { loadConfiguration } from './config_loader';

export const initApm = (
argv: string[],
rootDir: string,
isDistributable: boolean,
serviceName: string
) => {
const apmConfigLoader = loadConfiguration(argv, rootDir, isDistributable);
const apmConfig = apmConfigLoader.getConfig(serviceName);

// we want to only load the module when effectively used
// eslint-disable-next-line @typescript-eslint/no-var-requires
const apm = require('elastic-apm-node');

// Filter out all user PII
apm.addFilter((payload: Record<string, any>) => {
try {
if (payload.context?.user && typeof payload.context.user === 'object') {
Object.keys(payload.context.user).forEach((key) => {
payload.context.user[key] = '[REDACTED]';
});
}
} catch (e) {
// just silently ignore the error
}
return payload;
});

apm.start(apmConfig);
};
52 changes: 0 additions & 52 deletions src/apm.js

This file was deleted.

18 changes: 18 additions & 0 deletions src/cli/apm.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

const { join } = require('path');
const { name, build } = require('../../package.json');
const { initApm } = require('@kbn/apm-config-loader');

const rootDir = join(__dirname, '../..');
const isKibanaDistributable = Boolean(build && build.distributable === true);

module.exports = function (serviceName = name) {
initApm(process.argv, rootDir, isKibanaDistributable, serviceName);
};
2 changes: 1 addition & 1 deletion src/cli/dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

require('../apm')(process.env.ELASTIC_APM_SERVICE_NAME || 'kibana-proxy');
require('./apm')(process.env.ELASTIC_APM_SERVICE_NAME || 'kibana-proxy');
require('../setup_node_env');
require('../setup_node_env/root');
require('./cli');
2 changes: 1 addition & 1 deletion src/cli/dist.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

require('../apm')();
require('./apm')();
require('../setup_node_env/dist');
require('../setup_node_env/root');
require('./cli');
6 changes: 3 additions & 3 deletions src/core/server/http_resources/get_apm_config.test.mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* Side Public License, v 1.
*/

export const getConfigMock = jest.fn();
jest.doMock('../../../apm', () => ({
getConfig: getConfigMock,
export const getConfigurationMock = jest.fn();
jest.doMock('@kbn/apm-config-loader', () => ({
getConfiguration: getConfigurationMock,
}));

export const agentMock = {} as Record<string, any>;
Expand Down
12 changes: 6 additions & 6 deletions src/core/server/http_resources/get_apm_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import { getConfigMock, agentMock } from './get_apm_config.test.mocks';
import { getConfigurationMock, agentMock } from './get_apm_config.test.mocks';
import { getApmConfig } from './get_apm_config';

const defaultApmConfig = {
Expand All @@ -16,28 +16,28 @@ const defaultApmConfig = {

describe('getApmConfig', () => {
beforeEach(() => {
getConfigMock.mockReturnValue(defaultApmConfig);
getConfigurationMock.mockReturnValue(defaultApmConfig);
});

afterEach(() => {
getConfigMock.mockReset();
getConfigurationMock.mockReset();
agentMock.currentTransaction = null;
});

it('returns null if apm is disabled', () => {
getConfigMock.mockReturnValue({
getConfigurationMock.mockReturnValue({
active: false,
});
expect(getApmConfig('/path')).toBeNull();

getConfigMock.mockReturnValue(undefined);
getConfigurationMock.mockReturnValue(undefined);
expect(getApmConfig('/path')).toBeNull();
});

it('calls `getConfig` with the correct parameters', () => {
getApmConfig('/path');

expect(getConfigMock).toHaveBeenCalledWith('kibana-frontend');
expect(getConfigurationMock).toHaveBeenCalledWith('kibana-frontend');
});

it('returns the configuration from the `getConfig` call', () => {
Expand Down
5 changes: 2 additions & 3 deletions src/core/server/http_resources/get_apm_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,10 @@
*/

import agent from 'elastic-apm-node';
// @ts-expect-error apm module is a js file outside of core (need to split APM/rum configuration)
import { getConfig } from '../../../apm';
import { getConfiguration } from '@kbn/apm-config-loader';

export const getApmConfig = (requestPath: string) => {
const baseConfig = getConfig('kibana-frontend');
const baseConfig = getConfiguration('kibana-frontend');
if (!baseConfig?.active) {
return null;
}
Expand Down

0 comments on commit ed5bbda

Please sign in to comment.