From 441e9206f49a04f42ff7183a34250cdff00a7fb3 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 18 Sep 2020 10:45:27 +0200 Subject: [PATCH 01/12] first shot of the apm configuration loader --- config/kibana.yml | 2 +- package.json | 1 + packages/kbn-apm-config-loader/README.md | 13 ++ packages/kbn-apm-config-loader/package.json | 23 +++ packages/kbn-apm-config-loader/src/config.ts | 105 ++++++++++++ .../src/config_loader.ts | 33 ++++ packages/kbn-apm-config-loader/src/index.ts | 21 +++ .../src/utils/ensure_deep_object.test.ts | 156 ++++++++++++++++++ .../src/utils/ensure_deep_object.ts | 61 +++++++ .../src/utils/get_config_file_paths.ts | 45 +++++ .../kbn-apm-config-loader/src/utils/index.ts | 21 +++ .../src/utils/read_config.test.ts | 79 +++++++++ .../src/utils/read_config.ts | 64 +++++++ packages/kbn-apm-config-loader/tsconfig.json | 12 ++ packages/kbn-apm-config-loader/yarn.lock | 1 + src/apm.js | 71 +------- 16 files changed, 644 insertions(+), 64 deletions(-) create mode 100644 packages/kbn-apm-config-loader/README.md create mode 100644 packages/kbn-apm-config-loader/package.json create mode 100644 packages/kbn-apm-config-loader/src/config.ts create mode 100644 packages/kbn-apm-config-loader/src/config_loader.ts create mode 100644 packages/kbn-apm-config-loader/src/index.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/ensure_deep_object.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/index.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/read_config.test.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/read_config.ts create mode 100644 packages/kbn-apm-config-loader/tsconfig.json create mode 120000 packages/kbn-apm-config-loader/yarn.lock diff --git a/config/kibana.yml b/config/kibana.yml index 72e0764f849a0..81573956eec36 100644 --- a/config/kibana.yml +++ b/config/kibana.yml @@ -16,7 +16,7 @@ # `server.basePath` or require that they are rewritten by your reverse proxy. # This setting was effectively always `false` before Kibana 6.3 and will # default to `true` starting in Kibana 7.0. -#server.rewriteBasePath: false +server.rewriteBasePath: false # The maximum payload size in bytes for incoming server requests. #server.maxPayloadBytes: 1048576 diff --git a/package.json b/package.json index c88fbc0e5fd07..b9332a347e450 100644 --- a/package.json +++ b/package.json @@ -127,6 +127,7 @@ "@hapi/good-squeeze": "5.2.1", "@hapi/wreck": "^15.0.2", "@kbn/analytics": "1.0.0", + "@kbn/apm-config-loader": "1.0.0", "@kbn/babel-preset": "1.0.0", "@kbn/config": "1.0.0", "@kbn/config-schema": "1.0.0", diff --git a/packages/kbn-apm-config-loader/README.md b/packages/kbn-apm-config-loader/README.md new file mode 100644 index 0000000000000..75a57239b702d --- /dev/null +++ b/packages/kbn-apm-config-loader/README.md @@ -0,0 +1,13 @@ +# @kbn/apm-config-loader + +Configuration loader for the APM instrumentation script. + +This module is only meant to be used by the APM instrumentation script (`src/apm.js`) +to load the required configuration options from the `kibana.yaml` configuration file with +default values. + +### Why can't just use @kbn-config? + +`@kbn/config` is the recommended way to load and read the kibana configuration file, +however in the specific case of APM, we want to only need the minimal dependencies +before loading `elastic-apm-node` to avoid loosing instrumentation on the already loaded modules. \ No newline at end of file diff --git a/packages/kbn-apm-config-loader/package.json b/packages/kbn-apm-config-loader/package.json new file mode 100644 index 0000000000000..1982ccdeda0ff --- /dev/null +++ b/packages/kbn-apm-config-loader/package.json @@ -0,0 +1,23 @@ +{ + "name": "@kbn/apm-config-loader", + "main": "./target/index.js", + "types": "./target/index.d.ts", + "version": "1.0.0", + "license": "Apache-2.0", + "private": true, + "scripts": { + "build": "tsc", + "kbn:bootstrap": "yarn build", + "kbn:watch": "yarn build --watch" + }, + "dependencies": { + "@elastic/safer-lodash-set": "0.0.0", + "@kbn/utils": "1.0.0", + "js-yaml": "3.13.1", + "lodash": "^4.17.20" + }, + "devDependencies": { + "typescript": "4.0.2", + "tsd": "^0.7.4" + } +} diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts new file mode 100644 index 0000000000000..66c87b3ca59db --- /dev/null +++ b/packages/kbn-apm-config-loader/src/config.ts @@ -0,0 +1,105 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { join } from 'path'; +import { merge } from 'lodash'; +import { execSync } from 'child_process'; +// deep import to avoid loading the whole package +import { getDataPath } from '@kbn/utils/target/path'; +import { readFileSync } from 'fs'; + +const defaultConfig = { + active: false, + serverUrl: 'https://f1542b814f674090afd914960583265f.apm.us-central1.gcp.cloud.es.io:443', + // The secretToken below is intended to be hardcoded in this file even though + // it makes it public. This is not a security/privacy issue. Normally we'd + // instead disable the need for a secretToken in the APM Server config where + // the data is transmitted to, but due to how it's being hosted, it's easier, + // for now, to simply leave it in. + secretToken: 'R0Gjg46pE9K9wGestd', + globalLabels: {}, + breakdownMetrics: true, + centralConfig: false, + logUncaughtExceptions: true, +}; + +export class ApmConfiguration { + constructor( + private readonly rootDir: string, + private readonly rawKibanaConfig: Record + ) {} + + public getConfig(serviceName: string) { + const apmConfig = merge(defaultConfig, this.getDevConfig()); + + const rev = this.getGitRev(); + if (rev !== null) { + apmConfig.globalLabels.git_rev = rev; + } + + const uuid = this.getKibanaUuid(); + if (uuid) { + apmConfig.globalLabels.kibana_uuid = uuid; + } + + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { version } = require(join(this.rootDir, 'package.json')); + + return { + ...apmConfig, + serviceName: `${serviceName}-${version.replace(/\./g, '_')}`, + }; + } + + private getKibanaUuid() { + // try to access the `server.uuid` value from the config file first. + // if not manually defined, we will then read the value from the `{DATA_FOLDER}/uuid` file. + // note that as the file is created by the platform AFTER apm init, the file + // will not be present at first startup, but there is nothing we can really do about that. + if (this.rawKibanaConfig.server?.uuid) { + return this.rawKibanaConfig.server?.uuid; + } + + const dataPath: string = this.rawKibanaConfig.path?.data || getDataPath(); + try { + const filename = join(dataPath, 'uuid'); + return readFileSync(filename, 'utf-8'); + } catch (e) {} // eslint-disable-line no-empty + } + + private getDevConfig() { + try { + const apmDevConfigPath = join(this.rootDir, 'config', 'apm.dev.js'); + return require(apmDevConfigPath); + } catch (e) { + return {}; + } + } + + private getGitRev() { + try { + return execSync('git rev-parse --short HEAD', { + encoding: 'utf-8' as BufferEncoding, + stdio: ['ignore', 'pipe', 'ignore'], + }).trim(); + } catch (e) { + return null; + } + } +} diff --git a/packages/kbn-apm-config-loader/src/config_loader.ts b/packages/kbn-apm-config-loader/src/config_loader.ts new file mode 100644 index 0000000000000..5268ddc7c937a --- /dev/null +++ b/packages/kbn-apm-config-loader/src/config_loader.ts @@ -0,0 +1,33 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { getConfigurationFilePaths, getConfigFromFiles } from './utils'; + +import { ApmConfiguration } from './config'; + +/** + * Load the APM configuration. + * + * @param root The root directory of kibana (where the sources and the `package.json` file are) + */ +export const loadConfiguration = (rootDir: string): ApmConfiguration => { + const configPaths = getConfigurationFilePaths(); + const rawConfiguration = getConfigFromFiles(configPaths); + return new ApmConfiguration(rootDir, rawConfiguration); +}; diff --git a/packages/kbn-apm-config-loader/src/index.ts b/packages/kbn-apm-config-loader/src/index.ts new file mode 100644 index 0000000000000..179a2d42594ec --- /dev/null +++ b/packages/kbn-apm-config-loader/src/index.ts @@ -0,0 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { loadConfiguration } from './config_loader'; +export type { ApmConfiguration } from './config'; diff --git a/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts b/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts new file mode 100644 index 0000000000000..5a520fbeef316 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts @@ -0,0 +1,156 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ensureDeepObject } from './ensure_deep_object'; + +test('flat object', () => { + const obj = { + 'foo.a': 1, + 'foo.b': 2, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + a: 1, + b: 2, + }, + }); +}); + +test('deep object', () => { + const obj = { + foo: { + a: 1, + b: 2, + }, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + a: 1, + b: 2, + }, + }); +}); + +test('flat within deep object', () => { + const obj = { + foo: { + b: 2, + 'bar.a': 1, + }, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + b: 2, + bar: { + a: 1, + }, + }, + }); +}); + +test('flat then flat object', () => { + const obj = { + 'foo.bar': { + b: 2, + 'quux.a': 1, + }, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + bar: { + b: 2, + quux: { + a: 1, + }, + }, + }, + }); +}); + +test('full with empty array', () => { + const obj = { + a: 1, + b: [], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [], + }); +}); + +test('full with array of primitive values', () => { + const obj = { + a: 1, + b: [1, 2, 3], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [1, 2, 3], + }); +}); + +test('full with array of full objects', () => { + const obj = { + a: 1, + b: [{ c: 2 }, { d: 3 }], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [{ c: 2 }, { d: 3 }], + }); +}); + +test('full with array of flat objects', () => { + const obj = { + a: 1, + b: [{ 'c.d': 2 }, { 'e.f': 3 }], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [{ c: { d: 2 } }, { e: { f: 3 } }], + }); +}); + +test('flat with flat and array of flat objects', () => { + const obj = { + a: 1, + 'b.c': 2, + d: [3, { 'e.f': 4 }, { 'g.h': 5 }], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: { c: 2 }, + d: [3, { e: { f: 4 } }, { g: { h: 5 } }], + }); +}); + +test('array composed of flat objects', () => { + const arr = [{ 'c.d': 2 }, { 'e.f': 3 }]; + + expect(ensureDeepObject(arr)).toEqual([{ c: { d: 2 } }, { e: { f: 3 } }]); +}); diff --git a/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.ts b/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.ts new file mode 100644 index 0000000000000..6eaaef983355c --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.ts @@ -0,0 +1,61 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +const separator = '.'; + +/** + * Recursively traverses through the object's properties and expands ones with + * dot-separated names into nested objects (eg. { a.b: 'c'} -> { a: { b: 'c' }). + * @param obj Object to traverse through. + * @returns Same object instance with expanded properties. + */ +export function ensureDeepObject(obj: any): any { + if (obj == null || typeof obj !== 'object') { + return obj; + } + + if (Array.isArray(obj)) { + return obj.map((item) => ensureDeepObject(item)); + } + + return Object.keys(obj).reduce((fullObject, propertyKey) => { + const propertyValue = obj[propertyKey]; + if (!propertyKey.includes(separator)) { + fullObject[propertyKey] = ensureDeepObject(propertyValue); + } else { + walk(fullObject, propertyKey.split(separator), propertyValue); + } + + return fullObject; + }, {} as any); +} + +function walk(obj: any, keys: string[], value: any) { + const key = keys.shift()!; + if (keys.length === 0) { + obj[key] = value; + return; + } + + if (obj[key] === undefined) { + obj[key] = {}; + } + + walk(obj[key], keys, ensureDeepObject(value)); +} diff --git a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts new file mode 100644 index 0000000000000..ec2db12279770 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts @@ -0,0 +1,45 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { resolve } from 'path'; +// deep import to avoid loading the whole package +import { getConfigPath } from '@kbn/utils/target/path'; + +/** + * Return the configuration files that needs to be loaded. + * + * This mimics the behavior of the `src/cli/serve/serve.js` cli script by reading + * `-c` and `--config` options from process.argv, and fallbacks to `@kbn/utils`'s `getConfigPath()` + */ +export const getConfigurationFilePaths = (): string[] => { + let configPaths: string[] = []; + + const args = process.argv; + for (let i = 0; i < args.length; i++) { + if ((args[i] === '-c' || args[i] === '--config') && args[i + 1]) { + configPaths.push(resolve(process.cwd(), args[++i])); + } + } + + if (!configPaths.length) { + configPaths = [getConfigPath()]; + } + + return configPaths; +}; diff --git a/packages/kbn-apm-config-loader/src/utils/index.ts b/packages/kbn-apm-config-loader/src/utils/index.ts new file mode 100644 index 0000000000000..278042e986fb6 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/index.ts @@ -0,0 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { getConfigFromFiles } from './read_config'; +export { getConfigurationFilePaths } from './get_config_file_paths'; diff --git a/packages/kbn-apm-config-loader/src/utils/read_config.test.ts b/packages/kbn-apm-config-loader/src/utils/read_config.test.ts new file mode 100644 index 0000000000000..89b73c5d4e26a --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/read_config.test.ts @@ -0,0 +1,79 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { relative, resolve } from 'path'; +import { getConfigFromFiles } from './read_config'; + +const fixtureFile = (name: string) => resolve(`${__dirname}/../../__fixtures__/${name}`); + +test('reads single yaml from file system and parses to json', () => { + const config = getConfigFromFiles([fixtureFile('config.yml')]); + + expect(config).toMatchSnapshot(); +}); + +test('returns a deep object', () => { + const config = getConfigFromFiles([fixtureFile('/config_flat.yml')]); + + expect(config).toMatchSnapshot(); +}); + +test('reads and merges multiple yaml files from file system and parses to json', () => { + const config = getConfigFromFiles([fixtureFile('/one.yml'), fixtureFile('/two.yml')]); + + expect(config).toMatchSnapshot(); +}); + +test('should inject an environment variable value when setting a value with ${ENV_VAR}', () => { + process.env.KBN_ENV_VAR1 = 'val1'; + process.env.KBN_ENV_VAR2 = 'val2'; + + const config = getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]); + + delete process.env.KBN_ENV_VAR1; + delete process.env.KBN_ENV_VAR2; + + expect(config).toMatchSnapshot(); +}); + +test('should throw an exception when referenced environment variable in a config value does not exist', () => { + expect(() => + getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]) + ).toThrowErrorMatchingSnapshot(); +}); + +describe('different cwd()', () => { + const originalCwd = process.cwd(); + const tempCwd = resolve(__dirname); + + beforeAll(() => process.chdir(tempCwd)); + afterAll(() => process.chdir(originalCwd)); + + test('resolves relative files based on the cwd', () => { + const relativePath = relative(tempCwd, fixtureFile('/one.yml')); + const config = getConfigFromFiles([relativePath]); + + expect(config).toMatchSnapshot(); + }); + + test('fails to load relative paths, not found because of the cwd', () => { + const relativePath = relative(resolve(__dirname, '../../'), fixtureFile('/one.yml')); + expect(() => getConfigFromFiles([relativePath])).toThrowError(/ENOENT/); + }); +}); diff --git a/packages/kbn-apm-config-loader/src/utils/read_config.ts b/packages/kbn-apm-config-loader/src/utils/read_config.ts new file mode 100644 index 0000000000000..806366dc3e062 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/read_config.ts @@ -0,0 +1,64 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { readFileSync } from 'fs'; +import { safeLoad } from 'js-yaml'; + +import { set } from '@elastic/safer-lodash-set'; +import { isPlainObject } from 'lodash'; +import { ensureDeepObject } from './ensure_deep_object'; + +const readYaml = (path: string) => safeLoad(readFileSync(path, 'utf8')); + +function replaceEnvVarRefs(val: string) { + return val.replace(/\$\{(\w+)\}/g, (match, envVarName) => { + const envVarValue = process.env[envVarName]; + if (envVarValue !== undefined) { + return envVarValue; + } + + throw new Error(`Unknown environment variable referenced in config : ${envVarName}`); + }); +} + +function merge(target: Record, value: any, key?: string) { + if ((isPlainObject(value) || Array.isArray(value)) && Object.keys(value).length > 0) { + for (const [subKey, subVal] of Object.entries(value)) { + merge(target, subVal, key ? `${key}.${subKey}` : subKey); + } + } else if (key !== undefined) { + set(target, key, typeof value === 'string' ? replaceEnvVarRefs(value) : value); + } + + return target; +} + +/** @internal */ +export const getConfigFromFiles = (configFiles: readonly string[]) => { + let mergedYaml = {}; + + for (const configFile of configFiles) { + const yaml = readYaml(configFile); + if (yaml !== null) { + mergedYaml = merge(mergedYaml, yaml); + } + } + + return ensureDeepObject(mergedYaml); +}; diff --git a/packages/kbn-apm-config-loader/tsconfig.json b/packages/kbn-apm-config-loader/tsconfig.json new file mode 100644 index 0000000000000..ba00ddfa6adb6 --- /dev/null +++ b/packages/kbn-apm-config-loader/tsconfig.json @@ -0,0 +1,12 @@ +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "declaration": true, + "outDir": "./target", + "stripInternal": false, + "declarationMap": true, + "types": ["jest", "node"] + }, + "include": ["./src/**/*.ts"], + "exclude": ["target"] +} diff --git a/packages/kbn-apm-config-loader/yarn.lock b/packages/kbn-apm-config-loader/yarn.lock new file mode 120000 index 0000000000000..3f82ebc9cdbae --- /dev/null +++ b/packages/kbn-apm-config-loader/yarn.lock @@ -0,0 +1 @@ +../../yarn.lock \ No newline at end of file diff --git a/src/apm.js b/src/apm.js index effa6c77d7614..b9c4bc7371cea 100644 --- a/src/apm.js +++ b/src/apm.js @@ -18,67 +18,11 @@ */ const { join } = require('path'); -const { readFileSync } = require('fs'); -const { execSync } = require('child_process'); -const { merge } = require('lodash'); -const { name, version, build } = require('../package.json'); +const { name, build } = require('../package.json'); +const { loadConfiguration } = require('@kbn/apm-config-loader'); const ROOT_DIR = join(__dirname, '..'); - -function gitRev() { - try { - return execSync('git rev-parse --short HEAD', { - encoding: 'utf-8', - stdio: ['ignore', 'pipe', 'ignore'], - }).trim(); - } catch (e) { - return null; - } -} - -function devConfig() { - try { - const apmDevConfigPath = join(ROOT_DIR, 'config', 'apm.dev.js'); - return require(apmDevConfigPath); // eslint-disable-line import/no-dynamic-require - } catch (e) { - return {}; - } -} - -const apmConfig = merge( - { - active: false, - serverUrl: 'https://f1542b814f674090afd914960583265f.apm.us-central1.gcp.cloud.es.io:443', - // The secretToken below is intended to be hardcoded in this file even though - // it makes it public. This is not a security/privacy issue. Normally we'd - // instead disable the need for a secretToken in the APM Server config where - // the data is transmitted to, but due to how it's being hosted, it's easier, - // for now, to simply leave it in. - secretToken: 'R0Gjg46pE9K9wGestd', - globalLabels: {}, - breakdownMetrics: true, - centralConfig: false, - logUncaughtExceptions: true, - }, - devConfig() -); - -try { - const filename = join(ROOT_DIR, 'data', 'uuid'); - apmConfig.globalLabels.kibana_uuid = readFileSync(filename, 'utf-8'); -} catch (e) {} // eslint-disable-line no-empty - -const rev = gitRev(); -if (rev !== null) apmConfig.globalLabels.git_rev = rev; - -function getConfig(serviceName) { - return { - ...apmConfig, - ...{ - serviceName: `${serviceName}-${version.replace(/\./g, '_')}`, - }, - }; -} +const apmConfig = loadConfiguration(ROOT_DIR); /** * Flag to disable APM RUM support on all kibana builds by default @@ -86,12 +30,13 @@ function getConfig(serviceName) { const isKibanaDistributable = Boolean(build && build.distributable === true); module.exports = function (serviceName = name) { - if (process.env.kbnWorkerType === 'optmzr') return; - - const conf = getConfig(serviceName); + if (process.env.kbnWorkerType === 'optmzr') { + return; + } + const conf = apmConfig.getConfig(serviceName); require('elastic-apm-node').start(conf); }; -module.exports.getConfig = getConfig; +module.exports.getConfig = apmConfig.getConfig; module.exports.isKibanaDistributable = isKibanaDistributable; From a06f9aa7fd43c08beaecd37b5442e030422ce42d Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 18 Sep 2020 10:48:56 +0200 Subject: [PATCH 02/12] revert changes to kibana config --- config/kibana.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/kibana.yml b/config/kibana.yml index 81573956eec36..72e0764f849a0 100644 --- a/config/kibana.yml +++ b/config/kibana.yml @@ -16,7 +16,7 @@ # `server.basePath` or require that they are rewritten by your reverse proxy. # This setting was effectively always `false` before Kibana 6.3 and will # default to `true` starting in Kibana 7.0. -server.rewriteBasePath: false +#server.rewriteBasePath: false # The maximum payload size in bytes for incoming server requests. #server.maxPayloadBytes: 1048576 From 15671fd71512a14b4eb704434c1bec056a90f21c Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 18 Sep 2020 11:14:39 +0200 Subject: [PATCH 03/12] remove test files for now --- .../src/utils/ensure_deep_object.test.ts | 156 ------------------ .../src/utils/read_config.test.ts | 79 --------- 2 files changed, 235 deletions(-) delete mode 100644 packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts delete mode 100644 packages/kbn-apm-config-loader/src/utils/read_config.test.ts diff --git a/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts b/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts deleted file mode 100644 index 5a520fbeef316..0000000000000 --- a/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts +++ /dev/null @@ -1,156 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { ensureDeepObject } from './ensure_deep_object'; - -test('flat object', () => { - const obj = { - 'foo.a': 1, - 'foo.b': 2, - }; - - expect(ensureDeepObject(obj)).toEqual({ - foo: { - a: 1, - b: 2, - }, - }); -}); - -test('deep object', () => { - const obj = { - foo: { - a: 1, - b: 2, - }, - }; - - expect(ensureDeepObject(obj)).toEqual({ - foo: { - a: 1, - b: 2, - }, - }); -}); - -test('flat within deep object', () => { - const obj = { - foo: { - b: 2, - 'bar.a': 1, - }, - }; - - expect(ensureDeepObject(obj)).toEqual({ - foo: { - b: 2, - bar: { - a: 1, - }, - }, - }); -}); - -test('flat then flat object', () => { - const obj = { - 'foo.bar': { - b: 2, - 'quux.a': 1, - }, - }; - - expect(ensureDeepObject(obj)).toEqual({ - foo: { - bar: { - b: 2, - quux: { - a: 1, - }, - }, - }, - }); -}); - -test('full with empty array', () => { - const obj = { - a: 1, - b: [], - }; - - expect(ensureDeepObject(obj)).toEqual({ - a: 1, - b: [], - }); -}); - -test('full with array of primitive values', () => { - const obj = { - a: 1, - b: [1, 2, 3], - }; - - expect(ensureDeepObject(obj)).toEqual({ - a: 1, - b: [1, 2, 3], - }); -}); - -test('full with array of full objects', () => { - const obj = { - a: 1, - b: [{ c: 2 }, { d: 3 }], - }; - - expect(ensureDeepObject(obj)).toEqual({ - a: 1, - b: [{ c: 2 }, { d: 3 }], - }); -}); - -test('full with array of flat objects', () => { - const obj = { - a: 1, - b: [{ 'c.d': 2 }, { 'e.f': 3 }], - }; - - expect(ensureDeepObject(obj)).toEqual({ - a: 1, - b: [{ c: { d: 2 } }, { e: { f: 3 } }], - }); -}); - -test('flat with flat and array of flat objects', () => { - const obj = { - a: 1, - 'b.c': 2, - d: [3, { 'e.f': 4 }, { 'g.h': 5 }], - }; - - expect(ensureDeepObject(obj)).toEqual({ - a: 1, - b: { c: 2 }, - d: [3, { e: { f: 4 } }, { g: { h: 5 } }], - }); -}); - -test('array composed of flat objects', () => { - const arr = [{ 'c.d': 2 }, { 'e.f': 3 }]; - - expect(ensureDeepObject(arr)).toEqual([{ c: { d: 2 } }, { e: { f: 3 } }]); -}); diff --git a/packages/kbn-apm-config-loader/src/utils/read_config.test.ts b/packages/kbn-apm-config-loader/src/utils/read_config.test.ts deleted file mode 100644 index 89b73c5d4e26a..0000000000000 --- a/packages/kbn-apm-config-loader/src/utils/read_config.test.ts +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Licensed to Elasticsearch B.V. under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch B.V. licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -import { relative, resolve } from 'path'; -import { getConfigFromFiles } from './read_config'; - -const fixtureFile = (name: string) => resolve(`${__dirname}/../../__fixtures__/${name}`); - -test('reads single yaml from file system and parses to json', () => { - const config = getConfigFromFiles([fixtureFile('config.yml')]); - - expect(config).toMatchSnapshot(); -}); - -test('returns a deep object', () => { - const config = getConfigFromFiles([fixtureFile('/config_flat.yml')]); - - expect(config).toMatchSnapshot(); -}); - -test('reads and merges multiple yaml files from file system and parses to json', () => { - const config = getConfigFromFiles([fixtureFile('/one.yml'), fixtureFile('/two.yml')]); - - expect(config).toMatchSnapshot(); -}); - -test('should inject an environment variable value when setting a value with ${ENV_VAR}', () => { - process.env.KBN_ENV_VAR1 = 'val1'; - process.env.KBN_ENV_VAR2 = 'val2'; - - const config = getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]); - - delete process.env.KBN_ENV_VAR1; - delete process.env.KBN_ENV_VAR2; - - expect(config).toMatchSnapshot(); -}); - -test('should throw an exception when referenced environment variable in a config value does not exist', () => { - expect(() => - getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]) - ).toThrowErrorMatchingSnapshot(); -}); - -describe('different cwd()', () => { - const originalCwd = process.cwd(); - const tempCwd = resolve(__dirname); - - beforeAll(() => process.chdir(tempCwd)); - afterAll(() => process.chdir(originalCwd)); - - test('resolves relative files based on the cwd', () => { - const relativePath = relative(tempCwd, fixtureFile('/one.yml')); - const config = getConfigFromFiles([relativePath]); - - expect(config).toMatchSnapshot(); - }); - - test('fails to load relative paths, not found because of the cwd', () => { - const relativePath = relative(resolve(__dirname, '../../'), fixtureFile('/one.yml')); - expect(() => getConfigFromFiles([relativePath])).toThrowError(/ENOENT/); - }); -}); From d0162abdc6effce4cc37297d3f33c8cec16090e6 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 18 Sep 2020 11:17:11 +0200 Subject: [PATCH 04/12] remove `?.` usages --- packages/kbn-apm-config-loader/src/config.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts index 66c87b3ca59db..003eb431b111b 100644 --- a/packages/kbn-apm-config-loader/src/config.ts +++ b/packages/kbn-apm-config-loader/src/config.ts @@ -18,7 +18,7 @@ */ import { join } from 'path'; -import { merge } from 'lodash'; +import { merge, get } from 'lodash'; import { execSync } from 'child_process'; // deep import to avoid loading the whole package import { getDataPath } from '@kbn/utils/target/path'; @@ -72,11 +72,11 @@ export class ApmConfiguration { // if not manually defined, we will then read the value from the `{DATA_FOLDER}/uuid` file. // note that as the file is created by the platform AFTER apm init, the file // will not be present at first startup, but there is nothing we can really do about that. - if (this.rawKibanaConfig.server?.uuid) { - return this.rawKibanaConfig.server?.uuid; + if (get(this.rawKibanaConfig, 'server.uuid')) { + return this.rawKibanaConfig.server.uuid; } - const dataPath: string = this.rawKibanaConfig.path?.data || getDataPath(); + const dataPath: string = get(this.rawKibanaConfig, 'path.data') || getDataPath(); try { const filename = join(dataPath, 'uuid'); return readFileSync(filename, 'utf-8'); From 6927b293779396bed3af123885fdcab31fa55d76 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 22 Sep 2020 11:26:33 +0200 Subject: [PATCH 05/12] use lazy config init to avoid crashing integration test runner --- src/apm.js | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/apm.js b/src/apm.js index b9c4bc7371cea..c72bd00fb2009 100644 --- a/src/apm.js +++ b/src/apm.js @@ -22,7 +22,7 @@ const { name, build } = require('../package.json'); const { loadConfiguration } = require('@kbn/apm-config-loader'); const ROOT_DIR = join(__dirname, '..'); -const apmConfig = loadConfiguration(ROOT_DIR); +let apmConfig; /** * Flag to disable APM RUM support on all kibana builds by default @@ -34,9 +34,20 @@ module.exports = function (serviceName = name) { return; } + apmConfig = loadConfiguration(ROOT_DIR); const conf = apmConfig.getConfig(serviceName); require('elastic-apm-node').start(conf); }; -module.exports.getConfig = apmConfig.getConfig; +module.exports.getConfig = (serviceName) => { + // 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 {}; +}; module.exports.isKibanaDistributable = isKibanaDistributable; From 9436580c99ffb05e5ff578378c0f6ecc05499881 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 22 Sep 2020 11:49:39 +0200 Subject: [PATCH 06/12] loader improvements --- packages/kbn-apm-config-loader/src/config.ts | 43 ++++++++++++------- .../src/config_loader.ts | 7 +-- .../src/utils/get_config_file_paths.ts | 20 +++------ .../src/utils/read_argv.ts | 36 ++++++++++++++++ src/apm.js | 2 +- 5 files changed, 74 insertions(+), 34 deletions(-) create mode 100644 packages/kbn-apm-config-loader/src/utils/read_argv.ts diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts index 003eb431b111b..f23211eab4f68 100644 --- a/packages/kbn-apm-config-loader/src/config.ts +++ b/packages/kbn-apm-config-loader/src/config.ts @@ -40,31 +40,42 @@ const defaultConfig = { }; export class ApmConfiguration { + private baseConfig?: any; + private kibanaVersion: string; + constructor( private readonly rootDir: string, private readonly rawKibanaConfig: Record - ) {} + ) { + // eslint-disable-next-line @typescript-eslint/no-var-requires + const { version } = require(join(this.rootDir, 'package.json')); + this.kibanaVersion = version.replace(/\./g, '_'); + } public getConfig(serviceName: string) { - const apmConfig = merge(defaultConfig, this.getDevConfig()); + return { + ...this.getBaseConfig(), + serviceName: `${serviceName}-${this.kibanaVersion}`, + }; + } - const rev = this.getGitRev(); - if (rev !== null) { - apmConfig.globalLabels.git_rev = rev; - } + private getBaseConfig() { + if (!this.baseConfig) { + const apmConfig = merge(defaultConfig, this.getDevConfig()); - const uuid = this.getKibanaUuid(); - if (uuid) { - apmConfig.globalLabels.kibana_uuid = uuid; - } + const rev = this.getGitRev(); + if (rev !== null) { + apmConfig.globalLabels.git_rev = rev; + } - // eslint-disable-next-line @typescript-eslint/no-var-requires - const { version } = require(join(this.rootDir, 'package.json')); + const uuid = this.getKibanaUuid(); + if (uuid) { + apmConfig.globalLabels.kibana_uuid = uuid; + } + this.baseConfig = apmConfig; + } - return { - ...apmConfig, - serviceName: `${serviceName}-${version.replace(/\./g, '_')}`, - }; + return this.baseConfig; } private getKibanaUuid() { diff --git a/packages/kbn-apm-config-loader/src/config_loader.ts b/packages/kbn-apm-config-loader/src/config_loader.ts index 5268ddc7c937a..94bbd98df930b 100644 --- a/packages/kbn-apm-config-loader/src/config_loader.ts +++ b/packages/kbn-apm-config-loader/src/config_loader.ts @@ -24,10 +24,11 @@ import { ApmConfiguration } from './config'; /** * Load the APM configuration. * - * @param root The root directory of kibana (where the sources and the `package.json` file are) + * @param argv the `process.argv` arguments + * @param rootDir The root directory of kibana (where the sources and the `package.json` file are) */ -export const loadConfiguration = (rootDir: string): ApmConfiguration => { - const configPaths = getConfigurationFilePaths(); +export const loadConfiguration = (argv: string[], rootDir: string): ApmConfiguration => { + const configPaths = getConfigurationFilePaths(argv); const rawConfiguration = getConfigFromFiles(configPaths); return new ApmConfiguration(rootDir, rawConfiguration); }; diff --git a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts index ec2db12279770..85e3d5284d64f 100644 --- a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts +++ b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts @@ -20,6 +20,7 @@ import { resolve } from 'path'; // deep import to avoid loading the whole package import { getConfigPath } from '@kbn/utils/target/path'; +import { getFlagValues } from './read_argv'; /** * Return the configuration files that needs to be loaded. @@ -27,19 +28,10 @@ import { getConfigPath } from '@kbn/utils/target/path'; * This mimics the behavior of the `src/cli/serve/serve.js` cli script by reading * `-c` and `--config` options from process.argv, and fallbacks to `@kbn/utils`'s `getConfigPath()` */ -export const getConfigurationFilePaths = (): string[] => { - let configPaths: string[] = []; - - const args = process.argv; - for (let i = 0; i < args.length; i++) { - if ((args[i] === '-c' || args[i] === '--config') && args[i + 1]) { - configPaths.push(resolve(process.cwd(), args[++i])); - } - } - - if (!configPaths.length) { - configPaths = [getConfigPath()]; +export const getConfigurationFilePaths = (argv: string[]): string[] => { + const rawPaths = getFlagValues(argv, ['-c', '--config']); + if (rawPaths.length) { + return rawPaths.map((path) => resolve(process.cwd(), path)); } - - return configPaths; + return [getConfigPath()]; }; diff --git a/packages/kbn-apm-config-loader/src/utils/read_argv.ts b/packages/kbn-apm-config-loader/src/utils/read_argv.ts new file mode 100644 index 0000000000000..7602e45b4d16a --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/read_argv.ts @@ -0,0 +1,36 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export const getFlagValues = (argv: string[], flag: string | string[]): string[] => { + const flags = typeof flag === 'string' ? [flag] : flag; + const values: string[] = []; + for (let i = 0; i < argv.length; i++) { + if (flags.includes(argv[i]) && argv[i + 1]) { + values.push(argv[++i]); + } + } + return values; +}; + +export const getFlagValue = (argv: string[], flag: string | string[]): string | undefined => { + const values = getFlagValues(argv, flag); + if (values.length) { + return values[0]; + } +}; diff --git a/src/apm.js b/src/apm.js index c72bd00fb2009..f03f72b7cadf1 100644 --- a/src/apm.js +++ b/src/apm.js @@ -34,7 +34,7 @@ module.exports = function (serviceName = name) { return; } - apmConfig = loadConfiguration(ROOT_DIR); + apmConfig = loadConfiguration(process.argv, ROOT_DIR); const conf = apmConfig.getConfig(serviceName); require('elastic-apm-node').start(conf); }; From d64436eb9c5e67f62c340be6a8df2f578280d9cc Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Tue, 22 Sep 2020 12:17:13 +0200 Subject: [PATCH 07/12] add config value override via cli args --- .../src/config_loader.ts | 3 +- .../src/utils/apply_config_overrides.ts | 38 +++++++++++++++++++ .../kbn-apm-config-loader/src/utils/index.ts | 1 + .../src/utils/read_config.ts | 4 +- 4 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts diff --git a/packages/kbn-apm-config-loader/src/config_loader.ts b/packages/kbn-apm-config-loader/src/config_loader.ts index 94bbd98df930b..492ed38c85aef 100644 --- a/packages/kbn-apm-config-loader/src/config_loader.ts +++ b/packages/kbn-apm-config-loader/src/config_loader.ts @@ -17,7 +17,7 @@ * under the License. */ -import { getConfigurationFilePaths, getConfigFromFiles } from './utils'; +import { getConfigurationFilePaths, getConfigFromFiles, applyConfigOverrides } from './utils'; import { ApmConfiguration } from './config'; @@ -30,5 +30,6 @@ import { ApmConfiguration } from './config'; export const loadConfiguration = (argv: string[], rootDir: string): ApmConfiguration => { const configPaths = getConfigurationFilePaths(argv); const rawConfiguration = getConfigFromFiles(configPaths); + applyConfigOverrides(rawConfiguration, argv); return new ApmConfiguration(rootDir, rawConfiguration); }; diff --git a/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts b/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts new file mode 100644 index 0000000000000..acbdbf9c0829d --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts @@ -0,0 +1,38 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { set } from '@elastic/safer-lodash-set'; +import { getFlagValue } from './read_argv'; + +/** + * Manually applies the specific configuration overrides we need to load the APM config. + * Currently, only these are needed: + * - server.uuid + * - path.data + */ +export const applyConfigOverrides = (config: Record, argv: string[]) => { + const serverUuid = getFlagValue(argv, '--server.uuid'); + if (serverUuid) { + set(config, 'server.uuid', serverUuid); + } + const dataPath = getFlagValue(argv, '--path.data'); + if (dataPath) { + set(config, 'path.data', dataPath); + } +}; diff --git a/packages/kbn-apm-config-loader/src/utils/index.ts b/packages/kbn-apm-config-loader/src/utils/index.ts index 278042e986fb6..03a44e31a44d5 100644 --- a/packages/kbn-apm-config-loader/src/utils/index.ts +++ b/packages/kbn-apm-config-loader/src/utils/index.ts @@ -19,3 +19,4 @@ export { getConfigFromFiles } from './read_config'; export { getConfigurationFilePaths } from './get_config_file_paths'; +export { applyConfigOverrides } from './apply_config_overrides'; diff --git a/packages/kbn-apm-config-loader/src/utils/read_config.ts b/packages/kbn-apm-config-loader/src/utils/read_config.ts index 806366dc3e062..825bfd60181bf 100644 --- a/packages/kbn-apm-config-loader/src/utils/read_config.ts +++ b/packages/kbn-apm-config-loader/src/utils/read_config.ts @@ -50,8 +50,8 @@ function merge(target: Record, value: any, key?: string) { } /** @internal */ -export const getConfigFromFiles = (configFiles: readonly string[]) => { - let mergedYaml = {}; +export const getConfigFromFiles = (configFiles: readonly string[]): Record => { + let mergedYaml: Record = {}; for (const configFile of configFiles) { const yaml = readYaml(configFile); From 34698946a43bc908020b9d1487ca5f9c52453a6e Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 23 Sep 2020 09:12:00 +0200 Subject: [PATCH 08/12] add tests for utils package --- .../__fixtures__/config.yml | 11 ++ .../__fixtures__/config_flat.yml | 6 + .../__fixtures__/en_var_ref_config.yml | 5 + .../__fixtures__/one.yml | 9 + .../__fixtures__/two.yml | 10 ++ .../__snapshots__/read_config.test.ts.snap | 108 ++++++++++++ .../src/utils/apply_config_overrides.test.ts | 58 +++++++ .../src/utils/apply_config_overrides.ts | 6 +- .../src/utils/ensure_deep_object.test.ts | 156 ++++++++++++++++++ .../src/utils/get_config_file_paths.test.ts | 38 +++++ .../src/utils/get_config_file_paths.ts | 4 +- .../src/utils/read_argv.test.ts | 80 +++++++++ .../src/utils/read_argv.ts | 6 +- .../src/utils/read_config.test.ts | 79 +++++++++ 14 files changed, 568 insertions(+), 8 deletions(-) create mode 100644 packages/kbn-apm-config-loader/__fixtures__/config.yml create mode 100644 packages/kbn-apm-config-loader/__fixtures__/config_flat.yml create mode 100644 packages/kbn-apm-config-loader/__fixtures__/en_var_ref_config.yml create mode 100644 packages/kbn-apm-config-loader/__fixtures__/one.yml create mode 100644 packages/kbn-apm-config-loader/__fixtures__/two.yml create mode 100644 packages/kbn-apm-config-loader/src/utils/__snapshots__/read_config.test.ts.snap create mode 100644 packages/kbn-apm-config-loader/src/utils/apply_config_overrides.test.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/read_argv.test.ts create mode 100644 packages/kbn-apm-config-loader/src/utils/read_config.test.ts diff --git a/packages/kbn-apm-config-loader/__fixtures__/config.yml b/packages/kbn-apm-config-loader/__fixtures__/config.yml new file mode 100644 index 0000000000000..b0706d8ff8ea0 --- /dev/null +++ b/packages/kbn-apm-config-loader/__fixtures__/config.yml @@ -0,0 +1,11 @@ +pid: + enabled: true + file: '/var/run/kibana.pid' + obj: { val: 3 } + arr: [1] + empty_obj: {} + empty_arr: [] +obj: { val: 3 } +arr: [1, 2] +empty_obj: {} +empty_arr: [] diff --git a/packages/kbn-apm-config-loader/__fixtures__/config_flat.yml b/packages/kbn-apm-config-loader/__fixtures__/config_flat.yml new file mode 100644 index 0000000000000..a687a9a9088bf --- /dev/null +++ b/packages/kbn-apm-config-loader/__fixtures__/config_flat.yml @@ -0,0 +1,6 @@ +pid.enabled: true +pid.file: '/var/run/kibana.pid' +pid.obj: { val: 3 } +pid.arr: [1, 2] +pid.empty_obj: {} +pid.empty_arr: [] diff --git a/packages/kbn-apm-config-loader/__fixtures__/en_var_ref_config.yml b/packages/kbn-apm-config-loader/__fixtures__/en_var_ref_config.yml new file mode 100644 index 0000000000000..761f6a43ba452 --- /dev/null +++ b/packages/kbn-apm-config-loader/__fixtures__/en_var_ref_config.yml @@ -0,0 +1,5 @@ +foo: 1 +bar: "pre-${KBN_ENV_VAR1}-mid-${KBN_ENV_VAR2}-post" + +elasticsearch: + requestHeadersWhitelist: ["${KBN_ENV_VAR1}", "${KBN_ENV_VAR2}"] diff --git a/packages/kbn-apm-config-loader/__fixtures__/one.yml b/packages/kbn-apm-config-loader/__fixtures__/one.yml new file mode 100644 index 0000000000000..ccef51b546194 --- /dev/null +++ b/packages/kbn-apm-config-loader/__fixtures__/one.yml @@ -0,0 +1,9 @@ +foo: 1 +bar: true +xyz: ['1', '2'] +empty_arr: [] +abc: + def: test + qwe: 1 + zyx: { val: 1 } +pom.bom: 3 diff --git a/packages/kbn-apm-config-loader/__fixtures__/two.yml b/packages/kbn-apm-config-loader/__fixtures__/two.yml new file mode 100644 index 0000000000000..a2ec41265d50f --- /dev/null +++ b/packages/kbn-apm-config-loader/__fixtures__/two.yml @@ -0,0 +1,10 @@ +foo: 2 +baz: bonkers +xyz: ['3', '4'] +arr: [1] +empty_arr: [] +abc: + ghi: test2 + qwe: 2 + zyx: {} +pom.mob: 4 diff --git a/packages/kbn-apm-config-loader/src/utils/__snapshots__/read_config.test.ts.snap b/packages/kbn-apm-config-loader/src/utils/__snapshots__/read_config.test.ts.snap new file mode 100644 index 0000000000000..afdce4e76d3f5 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/__snapshots__/read_config.test.ts.snap @@ -0,0 +1,108 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`different cwd() resolves relative files based on the cwd 1`] = ` +Object { + "abc": Object { + "def": "test", + "qwe": 1, + "zyx": Object { + "val": 1, + }, + }, + "bar": true, + "empty_arr": Array [], + "foo": 1, + "pom": Object { + "bom": 3, + }, + "xyz": Array [ + "1", + "2", + ], +} +`; + +exports[`reads and merges multiple yaml files from file system and parses to json 1`] = ` +Object { + "abc": Object { + "def": "test", + "ghi": "test2", + "qwe": 2, + "zyx": Object {}, + }, + "arr": Array [ + 1, + ], + "bar": true, + "baz": "bonkers", + "empty_arr": Array [], + "foo": 2, + "pom": Object { + "bom": 3, + "mob": 4, + }, + "xyz": Array [ + "3", + "4", + ], +} +`; + +exports[`reads single yaml from file system and parses to json 1`] = ` +Object { + "arr": Array [ + 1, + 2, + ], + "empty_arr": Array [], + "empty_obj": Object {}, + "obj": Object { + "val": 3, + }, + "pid": Object { + "arr": Array [ + 1, + ], + "empty_arr": Array [], + "empty_obj": Object {}, + "enabled": true, + "file": "/var/run/kibana.pid", + "obj": Object { + "val": 3, + }, + }, +} +`; + +exports[`returns a deep object 1`] = ` +Object { + "pid": Object { + "arr": Array [ + 1, + 2, + ], + "empty_arr": Array [], + "empty_obj": Object {}, + "enabled": true, + "file": "/var/run/kibana.pid", + "obj": Object { + "val": 3, + }, + }, +} +`; + +exports[`should inject an environment variable value when setting a value with \${ENV_VAR} 1`] = ` +Object { + "bar": "pre-val1-mid-val2-post", + "elasticsearch": Object { + "requestHeadersWhitelist": Array [ + "val1", + "val2", + ], + }, + "foo": 1, +} +`; + +exports[`should throw an exception when referenced environment variable in a config value does not exist 1`] = `"Unknown environment variable referenced in config : KBN_ENV_VAR1"`; diff --git a/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.test.ts b/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.test.ts new file mode 100644 index 0000000000000..1d86f7e1f6e8a --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.test.ts @@ -0,0 +1,58 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { applyConfigOverrides } from './apply_config_overrides'; + +describe('applyConfigOverrides', () => { + it('overrides `server.uuid` when provided as a command line argument', () => { + const config: Record = { + server: { + uuid: 'from-config', + }, + }; + const argv = ['--server.uuid', 'from-argv']; + + applyConfigOverrides(config, argv); + + expect(config.server.uuid).toEqual('from-argv'); + }); + + it('overrides `path.data` when provided as a command line argument', () => { + const config: Record = { + path: { + data: '/from/config', + }, + }; + const argv = ['--path.data', '/from/argv']; + + applyConfigOverrides(config, argv); + + expect(config.path.data).toEqual('/from/argv'); + }); + + it('properly set the overridden properties even if the parent object is not present in the config', () => { + const config: Record = {}; + const argv = ['--server.uuid', 'from-argv', '--path.data', '/data-path']; + + applyConfigOverrides(config, argv); + + expect(config.server.uuid).toEqual('from-argv'); + expect(config.path.data).toEqual('/data-path'); + }); +}); diff --git a/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts b/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts index acbdbf9c0829d..6a3bf95f9954d 100644 --- a/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts +++ b/packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts @@ -18,7 +18,7 @@ */ import { set } from '@elastic/safer-lodash-set'; -import { getFlagValue } from './read_argv'; +import { getArgValue } from './read_argv'; /** * Manually applies the specific configuration overrides we need to load the APM config. @@ -27,11 +27,11 @@ import { getFlagValue } from './read_argv'; * - path.data */ export const applyConfigOverrides = (config: Record, argv: string[]) => { - const serverUuid = getFlagValue(argv, '--server.uuid'); + const serverUuid = getArgValue(argv, '--server.uuid'); if (serverUuid) { set(config, 'server.uuid', serverUuid); } - const dataPath = getFlagValue(argv, '--path.data'); + const dataPath = getArgValue(argv, '--path.data'); if (dataPath) { set(config, 'path.data', dataPath); } diff --git a/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts b/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts new file mode 100644 index 0000000000000..5a520fbeef316 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/ensure_deep_object.test.ts @@ -0,0 +1,156 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { ensureDeepObject } from './ensure_deep_object'; + +test('flat object', () => { + const obj = { + 'foo.a': 1, + 'foo.b': 2, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + a: 1, + b: 2, + }, + }); +}); + +test('deep object', () => { + const obj = { + foo: { + a: 1, + b: 2, + }, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + a: 1, + b: 2, + }, + }); +}); + +test('flat within deep object', () => { + const obj = { + foo: { + b: 2, + 'bar.a': 1, + }, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + b: 2, + bar: { + a: 1, + }, + }, + }); +}); + +test('flat then flat object', () => { + const obj = { + 'foo.bar': { + b: 2, + 'quux.a': 1, + }, + }; + + expect(ensureDeepObject(obj)).toEqual({ + foo: { + bar: { + b: 2, + quux: { + a: 1, + }, + }, + }, + }); +}); + +test('full with empty array', () => { + const obj = { + a: 1, + b: [], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [], + }); +}); + +test('full with array of primitive values', () => { + const obj = { + a: 1, + b: [1, 2, 3], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [1, 2, 3], + }); +}); + +test('full with array of full objects', () => { + const obj = { + a: 1, + b: [{ c: 2 }, { d: 3 }], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [{ c: 2 }, { d: 3 }], + }); +}); + +test('full with array of flat objects', () => { + const obj = { + a: 1, + b: [{ 'c.d': 2 }, { 'e.f': 3 }], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: [{ c: { d: 2 } }, { e: { f: 3 } }], + }); +}); + +test('flat with flat and array of flat objects', () => { + const obj = { + a: 1, + 'b.c': 2, + d: [3, { 'e.f': 4 }, { 'g.h': 5 }], + }; + + expect(ensureDeepObject(obj)).toEqual({ + a: 1, + b: { c: 2 }, + d: [3, { e: { f: 4 } }, { g: { h: 5 } }], + }); +}); + +test('array composed of flat objects', () => { + const arr = [{ 'c.d': 2 }, { 'e.f': 3 }]; + + expect(ensureDeepObject(arr)).toEqual([{ c: { d: 2 } }, { e: { f: 3 } }]); +}); diff --git a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts new file mode 100644 index 0000000000000..e205421656fa5 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts @@ -0,0 +1,38 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { resolve } from 'path'; +import { getConfigPath } from '@kbn/utils'; +import { getConfigurationFilePaths } from './get_config_file_paths'; + +describe('getConfigurationFilePaths', () => { + const cwd = process.cwd(); + + it('retrieve the config file paths from the command line arguments', () => { + const argv = ['--config', '/some/path', '-c', '/some-other-path']; + expect(getConfigurationFilePaths(argv)).toEqual([ + resolve(cwd, '/some/path'), + resolve(cwd, '/some-other-path'), + ]); + }); + + it('fallbacks to `getConfigPath` value', () => { + expect(getConfigurationFilePaths([])).toEqual([getConfigPath()]); + }); +}); diff --git a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts index 85e3d5284d64f..262f0d1c8b3f5 100644 --- a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts +++ b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts @@ -20,7 +20,7 @@ import { resolve } from 'path'; // deep import to avoid loading the whole package import { getConfigPath } from '@kbn/utils/target/path'; -import { getFlagValues } from './read_argv'; +import { getArgValues } from './read_argv'; /** * Return the configuration files that needs to be loaded. @@ -29,7 +29,7 @@ import { getFlagValues } from './read_argv'; * `-c` and `--config` options from process.argv, and fallbacks to `@kbn/utils`'s `getConfigPath()` */ export const getConfigurationFilePaths = (argv: string[]): string[] => { - const rawPaths = getFlagValues(argv, ['-c', '--config']); + const rawPaths = getArgValues(argv, ['-c', '--config']); if (rawPaths.length) { return rawPaths.map((path) => resolve(process.cwd(), path)); } diff --git a/packages/kbn-apm-config-loader/src/utils/read_argv.test.ts b/packages/kbn-apm-config-loader/src/utils/read_argv.test.ts new file mode 100644 index 0000000000000..282810e71681e --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/read_argv.test.ts @@ -0,0 +1,80 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { getArgValue, getArgValues } from './read_argv'; + +describe('getArgValues', () => { + it('retrieve the arg value from the provided argv arguments', () => { + const argValues = getArgValues( + ['--config', 'my-config', '--foo', '-b', 'bar', '--config', 'other-config', '--baz'], + '--config' + ); + expect(argValues).toEqual(['my-config', 'other-config']); + }); + + it('accept aliases', () => { + const argValues = getArgValues( + ['--config', 'my-config', '--foo', '-b', 'bar', '-c', 'other-config', '--baz'], + ['--config', '-c'] + ); + expect(argValues).toEqual(['my-config', 'other-config']); + }); + + it('returns an empty array when the arg is not found', () => { + const argValues = getArgValues( + ['--config', 'my-config', '--foo', '-b', 'bar', '-c', 'other-config', '--baz'], + '--unicorn' + ); + expect(argValues).toEqual([]); + }); + + it('ignores the flag when no value is provided', () => { + const argValues = getArgValues( + ['-c', 'my-config', '--foo', '-b', 'bar', '--config'], + ['--config', '-c'] + ); + expect(argValues).toEqual(['my-config']); + }); +}); + +describe('getArgValue', () => { + it('retrieve the first arg value from the provided argv arguments', () => { + const argValues = getArgValue( + ['--config', 'my-config', '--foo', '-b', 'bar', '--config', 'other-config', '--baz'], + '--config' + ); + expect(argValues).toEqual('my-config'); + }); + + it('accept aliases', () => { + const argValues = getArgValue( + ['-c', 'my-config', '--foo', '-b', 'bar', '--config', 'other-config', '--baz'], + ['--config', '-c'] + ); + expect(argValues).toEqual('my-config'); + }); + + it('returns undefined the arg is not found', () => { + const argValues = getArgValue( + ['--config', 'my-config', '--foo', '-b', 'bar', '-c', 'other-config', '--baz'], + '--unicorn' + ); + expect(argValues).toBeUndefined(); + }); +}); diff --git a/packages/kbn-apm-config-loader/src/utils/read_argv.ts b/packages/kbn-apm-config-loader/src/utils/read_argv.ts index 7602e45b4d16a..9a74d5344a0fc 100644 --- a/packages/kbn-apm-config-loader/src/utils/read_argv.ts +++ b/packages/kbn-apm-config-loader/src/utils/read_argv.ts @@ -17,7 +17,7 @@ * under the License. */ -export const getFlagValues = (argv: string[], flag: string | string[]): string[] => { +export const getArgValues = (argv: string[], flag: string | string[]): string[] => { const flags = typeof flag === 'string' ? [flag] : flag; const values: string[] = []; for (let i = 0; i < argv.length; i++) { @@ -28,8 +28,8 @@ export const getFlagValues = (argv: string[], flag: string | string[]): string[] return values; }; -export const getFlagValue = (argv: string[], flag: string | string[]): string | undefined => { - const values = getFlagValues(argv, flag); +export const getArgValue = (argv: string[], flag: string | string[]): string | undefined => { + const values = getArgValues(argv, flag); if (values.length) { return values[0]; } diff --git a/packages/kbn-apm-config-loader/src/utils/read_config.test.ts b/packages/kbn-apm-config-loader/src/utils/read_config.test.ts new file mode 100644 index 0000000000000..89b73c5d4e26a --- /dev/null +++ b/packages/kbn-apm-config-loader/src/utils/read_config.test.ts @@ -0,0 +1,79 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { relative, resolve } from 'path'; +import { getConfigFromFiles } from './read_config'; + +const fixtureFile = (name: string) => resolve(`${__dirname}/../../__fixtures__/${name}`); + +test('reads single yaml from file system and parses to json', () => { + const config = getConfigFromFiles([fixtureFile('config.yml')]); + + expect(config).toMatchSnapshot(); +}); + +test('returns a deep object', () => { + const config = getConfigFromFiles([fixtureFile('/config_flat.yml')]); + + expect(config).toMatchSnapshot(); +}); + +test('reads and merges multiple yaml files from file system and parses to json', () => { + const config = getConfigFromFiles([fixtureFile('/one.yml'), fixtureFile('/two.yml')]); + + expect(config).toMatchSnapshot(); +}); + +test('should inject an environment variable value when setting a value with ${ENV_VAR}', () => { + process.env.KBN_ENV_VAR1 = 'val1'; + process.env.KBN_ENV_VAR2 = 'val2'; + + const config = getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]); + + delete process.env.KBN_ENV_VAR1; + delete process.env.KBN_ENV_VAR2; + + expect(config).toMatchSnapshot(); +}); + +test('should throw an exception when referenced environment variable in a config value does not exist', () => { + expect(() => + getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]) + ).toThrowErrorMatchingSnapshot(); +}); + +describe('different cwd()', () => { + const originalCwd = process.cwd(); + const tempCwd = resolve(__dirname); + + beforeAll(() => process.chdir(tempCwd)); + afterAll(() => process.chdir(originalCwd)); + + test('resolves relative files based on the cwd', () => { + const relativePath = relative(tempCwd, fixtureFile('/one.yml')); + const config = getConfigFromFiles([relativePath]); + + expect(config).toMatchSnapshot(); + }); + + test('fails to load relative paths, not found because of the cwd', () => { + const relativePath = relative(resolve(__dirname, '../../'), fixtureFile('/one.yml')); + expect(() => getConfigFromFiles([relativePath])).toThrowError(/ENOENT/); + }); +}); From ee3280c414d5628b78aa9481c6b1b80dce9b77eb Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 23 Sep 2020 10:11:49 +0200 Subject: [PATCH 09/12] add prod/dev config handling + loader tests --- packages/kbn-apm-config-loader/src/config.ts | 51 ++++++++----- .../src/config_loader.test.mocks.ts | 45 +++++++++++ .../src/config_loader.test.ts | 75 +++++++++++++++++++ .../src/config_loader.ts | 10 ++- packages/kbn-apm-config-loader/src/index.ts | 1 + packages/kbn-apm-config-loader/src/types.ts | 24 ++++++ src/apm.js | 2 +- 7 files changed, 187 insertions(+), 21 deletions(-) create mode 100644 packages/kbn-apm-config-loader/src/config_loader.test.mocks.ts create mode 100644 packages/kbn-apm-config-loader/src/config_loader.test.ts create mode 100644 packages/kbn-apm-config-loader/src/types.ts diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts index f23211eab4f68..dd20815ba6839 100644 --- a/packages/kbn-apm-config-loader/src/config.ts +++ b/packages/kbn-apm-config-loader/src/config.ts @@ -23,20 +23,28 @@ import { execSync } from 'child_process'; // deep import to avoid loading the whole package import { getDataPath } from '@kbn/utils/target/path'; import { readFileSync } from 'fs'; +import { ApmAgentConfig } from './types'; -const defaultConfig = { - active: false, - serverUrl: 'https://f1542b814f674090afd914960583265f.apm.us-central1.gcp.cloud.es.io:443', - // The secretToken below is intended to be hardcoded in this file even though - // it makes it public. This is not a security/privacy issue. Normally we'd - // instead disable the need for a secretToken in the APM Server config where - // the data is transmitted to, but due to how it's being hosted, it's easier, - // for now, to simply leave it in. - secretToken: 'R0Gjg46pE9K9wGestd', - globalLabels: {}, - breakdownMetrics: true, - centralConfig: false, - logUncaughtExceptions: true, +const getDefaultConfig = (isDistributable: boolean): ApmAgentConfig => { + if (isDistributable) { + return { + active: false, + }; + } + return { + active: false, + serverUrl: 'https://f1542b814f674090afd914960583265f.apm.us-central1.gcp.cloud.es.io:443', + // The secretToken below is intended to be hardcoded in this file even though + // it makes it public. This is not a security/privacy issue. Normally we'd + // instead disable the need for a secretToken in the APM Server config where + // the data is transmitted to, but due to how it's being hosted, it's easier, + // for now, to simply leave it in. + secretToken: 'R0Gjg46pE9K9wGestd', + globalLabels: {}, + breakdownMetrics: true, + centralConfig: false, + logUncaughtExceptions: true, + }; }; export class ApmConfiguration { @@ -45,14 +53,15 @@ export class ApmConfiguration { constructor( private readonly rootDir: string, - private readonly rawKibanaConfig: Record + private readonly rawKibanaConfig: Record, + private readonly isDistributable: boolean ) { // eslint-disable-next-line @typescript-eslint/no-var-requires const { version } = require(join(this.rootDir, 'package.json')); this.kibanaVersion = version.replace(/\./g, '_'); } - public getConfig(serviceName: string) { + public getConfig(serviceName: string): ApmAgentConfig { return { ...this.getBaseConfig(), serviceName: `${serviceName}-${this.kibanaVersion}`, @@ -61,7 +70,11 @@ export class ApmConfiguration { private getBaseConfig() { if (!this.baseConfig) { - const apmConfig = merge(defaultConfig, this.getDevConfig()); + const apmConfig = merge( + getDefaultConfig(this.isDistributable), + this.getConfigFromKibanaConfig(), + this.getDevConfig() + ); const rev = this.getGitRev(); if (rev !== null) { @@ -78,6 +91,10 @@ export class ApmConfiguration { return this.baseConfig; } + private getConfigFromKibanaConfig(): ApmAgentConfig { + return get(this.rawKibanaConfig, 'elastic.apm', {}); + } + private getKibanaUuid() { // try to access the `server.uuid` value from the config file first. // if not manually defined, we will then read the value from the `{DATA_FOLDER}/uuid` file. @@ -94,7 +111,7 @@ export class ApmConfiguration { } catch (e) {} // eslint-disable-line no-empty } - private getDevConfig() { + private getDevConfig(): ApmAgentConfig { try { const apmDevConfigPath = join(this.rootDir, 'config', 'apm.dev.js'); return require(apmDevConfigPath); diff --git a/packages/kbn-apm-config-loader/src/config_loader.test.mocks.ts b/packages/kbn-apm-config-loader/src/config_loader.test.mocks.ts new file mode 100644 index 0000000000000..74b50d9daf632 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/config_loader.test.mocks.ts @@ -0,0 +1,45 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export const getConfigurationFilePathsMock = jest.fn(); +jest.doMock('./utils/get_config_file_paths', () => ({ + getConfigurationFilePaths: getConfigurationFilePathsMock, +})); + +export const getConfigFromFilesMock = jest.fn(); +jest.doMock('./utils/read_config', () => ({ + getConfigFromFiles: getConfigFromFilesMock, +})); + +export const applyConfigOverridesMock = jest.fn(); +jest.doMock('./utils/apply_config_overrides', () => ({ + applyConfigOverrides: applyConfigOverridesMock, +})); + +export const ApmConfigurationMock = jest.fn(); +jest.doMock('./config', () => ({ + ApmConfiguration: ApmConfigurationMock, +})); + +export const resetAllMocks = () => { + getConfigurationFilePathsMock.mockReset(); + getConfigFromFilesMock.mockReset(); + applyConfigOverridesMock.mockReset(); + ApmConfigurationMock.mockReset(); +}; diff --git a/packages/kbn-apm-config-loader/src/config_loader.test.ts b/packages/kbn-apm-config-loader/src/config_loader.test.ts new file mode 100644 index 0000000000000..da59237de231e --- /dev/null +++ b/packages/kbn-apm-config-loader/src/config_loader.test.ts @@ -0,0 +1,75 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { + ApmConfigurationMock, + applyConfigOverridesMock, + getConfigFromFilesMock, + getConfigurationFilePathsMock, + resetAllMocks, +} from './config_loader.test.mocks'; + +import { loadConfiguration } from './config_loader'; + +describe('loadConfiguration', () => { + const argv = ['some', 'arbitrary', 'args']; + const rootDir = '/root/dir'; + const isDistributable = false; + + afterEach(() => { + resetAllMocks(); + }); + + it('calls `getConfigurationFilePaths` with the correct arguments', () => { + loadConfiguration(argv, rootDir, isDistributable); + expect(getConfigurationFilePathsMock).toHaveBeenCalledTimes(1); + expect(getConfigurationFilePathsMock).toHaveBeenCalledWith(argv); + }); + + it('calls `getConfigFromFiles` with the correct arguments', () => { + const configPaths = ['/path/to/config', '/path/to/other/config']; + getConfigurationFilePathsMock.mockReturnValue(configPaths); + + loadConfiguration(argv, rootDir, isDistributable); + expect(getConfigFromFilesMock).toHaveBeenCalledTimes(1); + expect(getConfigFromFilesMock).toHaveBeenCalledWith(configPaths); + }); + + it('calls `applyConfigOverrides` with the correct arguments', () => { + const config = { server: { uuid: 'uuid' } }; + getConfigFromFilesMock.mockReturnValue(config); + + loadConfiguration(argv, rootDir, isDistributable); + expect(applyConfigOverridesMock).toHaveBeenCalledTimes(1); + expect(applyConfigOverridesMock).toHaveBeenCalledWith(config, argv); + }); + + it('creates and return an `ApmConfiguration` instance', () => { + const apmInstance = { apmInstance: true }; + ApmConfigurationMock.mockImplementation(() => apmInstance); + + const config = { server: { uuid: 'uuid' } }; + getConfigFromFilesMock.mockReturnValue(config); + + const instance = loadConfiguration(argv, rootDir, isDistributable); + expect(ApmConfigurationMock).toHaveBeenCalledTimes(1); + expect(ApmConfigurationMock).toHaveBeenCalledWith(rootDir, config, isDistributable); + expect(instance).toBe(apmInstance); + }); +}); diff --git a/packages/kbn-apm-config-loader/src/config_loader.ts b/packages/kbn-apm-config-loader/src/config_loader.ts index 492ed38c85aef..edddd445b9b7a 100644 --- a/packages/kbn-apm-config-loader/src/config_loader.ts +++ b/packages/kbn-apm-config-loader/src/config_loader.ts @@ -18,7 +18,6 @@ */ import { getConfigurationFilePaths, getConfigFromFiles, applyConfigOverrides } from './utils'; - import { ApmConfiguration } from './config'; /** @@ -26,10 +25,15 @@ import { ApmConfiguration } from './config'; * * @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 */ -export const loadConfiguration = (argv: string[], rootDir: string): ApmConfiguration => { +export const loadConfiguration = ( + argv: string[], + rootDir: string, + isDistributable: boolean +): ApmConfiguration => { const configPaths = getConfigurationFilePaths(argv); const rawConfiguration = getConfigFromFiles(configPaths); applyConfigOverrides(rawConfiguration, argv); - return new ApmConfiguration(rootDir, rawConfiguration); + return new ApmConfiguration(rootDir, rawConfiguration, isDistributable); }; diff --git a/packages/kbn-apm-config-loader/src/index.ts b/packages/kbn-apm-config-loader/src/index.ts index 179a2d42594ec..0d9c057c7cf89 100644 --- a/packages/kbn-apm-config-loader/src/index.ts +++ b/packages/kbn-apm-config-loader/src/index.ts @@ -19,3 +19,4 @@ export { loadConfiguration } from './config_loader'; export type { ApmConfiguration } from './config'; +export type { ApmAgentConfig } from './types'; diff --git a/packages/kbn-apm-config-loader/src/types.ts b/packages/kbn-apm-config-loader/src/types.ts new file mode 100644 index 0000000000000..172edfe0af009 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/types.ts @@ -0,0 +1,24 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// There is an (incomplete) `AgentConfigOptions` type declared in node_modules/elastic-apm-node/index.d.ts +// but it's not exported, and using ts tricks to retrieve the type via Parameters[0] +// causes errors in the generated .d.ts file because of esModuleInterop and the fact that the apm module +// is just exporting an instance of the `ApmAgent` type. +export type ApmAgentConfig = Record; diff --git a/src/apm.js b/src/apm.js index f03f72b7cadf1..8a0c010d993f1 100644 --- a/src/apm.js +++ b/src/apm.js @@ -34,7 +34,7 @@ module.exports = function (serviceName = name) { return; } - apmConfig = loadConfiguration(process.argv, ROOT_DIR); + apmConfig = loadConfiguration(process.argv, ROOT_DIR, isKibanaDistributable); const conf = apmConfig.getConfig(serviceName); require('elastic-apm-node').start(conf); }; From 20b6925f5179fa44dee9fc390b5b2584f7e37fc9 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Wed, 23 Sep 2020 12:00:44 +0200 Subject: [PATCH 10/12] add tests for config --- .../src/config.test.mocks.ts | 66 ++++++++ .../kbn-apm-config-loader/src/config.test.ts | 143 ++++++++++++++++++ packages/kbn-apm-config-loader/src/config.ts | 1 + 3 files changed, 210 insertions(+) create mode 100644 packages/kbn-apm-config-loader/src/config.test.mocks.ts create mode 100644 packages/kbn-apm-config-loader/src/config.test.ts diff --git a/packages/kbn-apm-config-loader/src/config.test.mocks.ts b/packages/kbn-apm-config-loader/src/config.test.mocks.ts new file mode 100644 index 0000000000000..a0422665a55d2 --- /dev/null +++ b/packages/kbn-apm-config-loader/src/config.test.mocks.ts @@ -0,0 +1,66 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { join } from 'path'; +const childProcessModule = jest.requireActual('child_process'); +const fsModule = jest.requireActual('fs'); + +export const mockedRootDir = '/root'; + +export const packageMock = { + raw: {} as any, +}; +jest.doMock(join(mockedRootDir, 'package.json'), () => packageMock.raw, { virtual: true }); + +export const devConfigMock = { + raw: {} as any, +}; +jest.doMock(join(mockedRootDir, 'config', 'apm.dev.js'), () => devConfigMock.raw, { + virtual: true, +}); + +export const gitRevExecMock = jest.fn(); +jest.doMock('child_process', () => ({ + ...childProcessModule, + execSync: (command: string, options: any) => { + if (command.startsWith('git rev-parse')) { + return gitRevExecMock(command, options); + } + return childProcessModule.execSync(command, options); + }, +})); + +export const readUuidFileMock = jest.fn(); +jest.doMock('fs', () => ({ + ...fsModule, + readFileSync: (path: string, options: any) => { + if (path.endsWith('uuid')) { + return readUuidFileMock(path, options); + } + return fsModule.readFileSync(path, options); + }, +})); + +export const resetAllMocks = () => { + packageMock.raw = {}; + devConfigMock.raw = {}; + gitRevExecMock.mockReset(); + readUuidFileMock.mockReset(); + jest.resetModules(); +}; diff --git a/packages/kbn-apm-config-loader/src/config.test.ts b/packages/kbn-apm-config-loader/src/config.test.ts new file mode 100644 index 0000000000000..50ecbbf3a2c4e --- /dev/null +++ b/packages/kbn-apm-config-loader/src/config.test.ts @@ -0,0 +1,143 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { + packageMock, + mockedRootDir, + gitRevExecMock, + devConfigMock, + readUuidFileMock, + resetAllMocks, +} from './config.test.mocks'; + +import { ApmConfiguration } from './config'; + +describe('ApmConfiguration', () => { + beforeEach(() => { + packageMock.raw = { + version: '8.0.0', + }; + }); + + afterEach(() => { + resetAllMocks(); + }); + + it('sets the correct service name', () => { + packageMock.raw = { + version: '9.2.1', + }; + const config = new ApmConfiguration(mockedRootDir, {}, false); + expect(config.getConfig('myservice').serviceName).toBe('myservice-9_2_1'); + }); + + it('sets the giv revision in globalLabels', () => { + gitRevExecMock.mockReturnValue('some-git-rev'); + const config = new ApmConfiguration(mockedRootDir, {}, false); + expect(config.getConfig('serviceName').globalLabels.git_rev).toBe('some-git-rev'); + }); + + it('reads the kibana uuid from the uuid file', () => { + readUuidFileMock.mockReturnValue('instance-uuid'); + const config = new ApmConfiguration(mockedRootDir, {}, false); + expect(config.getConfig('serviceName').globalLabels.kibana_uuid).toBe('instance-uuid'); + }); + + it('uses the uuid from the kibana config if present', () => { + readUuidFileMock.mockReturnValue('uuid-from-file'); + const kibanaConfig = { + server: { + uuid: 'uuid-from-config', + }, + }; + const config = new ApmConfiguration(mockedRootDir, kibanaConfig, false); + expect(config.getConfig('serviceName').globalLabels.kibana_uuid).toBe('uuid-from-config'); + }); + + it('uses the correct default config depending on the `isDistributable` parameter', () => { + let config = new ApmConfiguration(mockedRootDir, {}, false); + expect(config.getConfig('serviceName')).toEqual( + expect.objectContaining({ + serverUrl: expect.any(String), + secretToken: expect.any(String), + }) + ); + + config = new ApmConfiguration(mockedRootDir, {}, true); + expect(Object.keys(config.getConfig('serviceName'))).not.toContain('serverUrl'); + }); + + it('loads the configuration from the kibana config file', () => { + const kibanaConfig = { + elastic: { + apm: { + active: true, + serverUrl: 'https://url', + secretToken: 'secret', + }, + }, + }; + const config = new ApmConfiguration(mockedRootDir, kibanaConfig, true); + expect(config.getConfig('serviceName')).toEqual( + expect.objectContaining({ + active: true, + serverUrl: 'https://url', + secretToken: 'secret', + }) + ); + }); + + it('loads the configuration from the dev config is present', () => { + devConfigMock.raw = { + active: true, + serverUrl: 'https://dev-url.co', + }; + const config = new ApmConfiguration(mockedRootDir, {}, true); + expect(config.getConfig('serviceName')).toEqual( + expect.objectContaining({ + active: true, + serverUrl: 'https://dev-url.co', + }) + ); + }); + + it('respect the precedence of the dev config', () => { + const kibanaConfig = { + elastic: { + apm: { + active: true, + serverUrl: 'https://url', + secretToken: 'secret', + }, + }, + }; + devConfigMock.raw = { + active: true, + serverUrl: 'https://dev-url.co', + }; + const config = new ApmConfiguration(mockedRootDir, kibanaConfig, true); + expect(config.getConfig('serviceName')).toEqual( + expect.objectContaining({ + active: true, + serverUrl: 'https://dev-url.co', + secretToken: 'secret', + }) + ); + }); +}); diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts index dd20815ba6839..651b9f05621a0 100644 --- a/packages/kbn-apm-config-loader/src/config.ts +++ b/packages/kbn-apm-config-loader/src/config.ts @@ -29,6 +29,7 @@ const getDefaultConfig = (isDistributable: boolean): ApmAgentConfig => { if (isDistributable) { return { active: false, + globalLabels: {}, }; } return { From 8fc031dc2a963906975a5f518dcee91afe6158b9 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 28 Sep 2020 08:53:49 +0200 Subject: [PATCH 11/12] address josh's comments --- packages/kbn-apm-config-loader/README.md | 2 +- .../kbn-apm-config-loader/src/config.test.ts | 17 ++++++++++++++++- packages/kbn-apm-config-loader/src/config.ts | 7 ++++++- .../src/utils/get_config_file_paths.test.ts | 9 +++++---- .../src/utils/read_config.test.ts | 14 +++++++------- 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/packages/kbn-apm-config-loader/README.md b/packages/kbn-apm-config-loader/README.md index 75a57239b702d..e5280cd36bab9 100644 --- a/packages/kbn-apm-config-loader/README.md +++ b/packages/kbn-apm-config-loader/README.md @@ -10,4 +10,4 @@ default values. `@kbn/config` is the recommended way to load and read the kibana configuration file, however in the specific case of APM, we want to only need the minimal dependencies -before loading `elastic-apm-node` to avoid loosing instrumentation on the already loaded modules. \ No newline at end of file +before loading `elastic-apm-node` to avoid losing instrumentation on the already loaded modules. \ No newline at end of file diff --git a/packages/kbn-apm-config-loader/src/config.test.ts b/packages/kbn-apm-config-loader/src/config.test.ts index 50ecbbf3a2c4e..fe6247673e312 100644 --- a/packages/kbn-apm-config-loader/src/config.test.ts +++ b/packages/kbn-apm-config-loader/src/config.test.ts @@ -32,6 +32,9 @@ describe('ApmConfiguration', () => { beforeEach(() => { packageMock.raw = { version: '8.0.0', + build: { + sha: 'sha', + }, }; }); @@ -47,12 +50,24 @@ describe('ApmConfiguration', () => { expect(config.getConfig('myservice').serviceName).toBe('myservice-9_2_1'); }); - it('sets the giv revision in globalLabels', () => { + it('sets the git revision from `git rev-parse` command in non distribution mode', () => { gitRevExecMock.mockReturnValue('some-git-rev'); const config = new ApmConfiguration(mockedRootDir, {}, false); expect(config.getConfig('serviceName').globalLabels.git_rev).toBe('some-git-rev'); }); + it('sets the git revision from `pkg.build.sha` in distribution mode', () => { + gitRevExecMock.mockReturnValue('dev-sha'); + packageMock.raw = { + version: '9.2.1', + build: { + sha: 'distribution-sha', + }, + }; + const config = new ApmConfiguration(mockedRootDir, {}, true); + expect(config.getConfig('serviceName').globalLabels.git_rev).toBe('distribution-sha'); + }); + it('reads the kibana uuid from the uuid file', () => { readUuidFileMock.mockReturnValue('instance-uuid'); const config = new ApmConfiguration(mockedRootDir, {}, false); diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts index 651b9f05621a0..aab82c6c06a58 100644 --- a/packages/kbn-apm-config-loader/src/config.ts +++ b/packages/kbn-apm-config-loader/src/config.ts @@ -51,6 +51,7 @@ const getDefaultConfig = (isDistributable: boolean): ApmAgentConfig => { export class ApmConfiguration { private baseConfig?: any; private kibanaVersion: string; + private pkgBuild: Record; constructor( private readonly rootDir: string, @@ -58,8 +59,9 @@ export class ApmConfiguration { private readonly isDistributable: boolean ) { // eslint-disable-next-line @typescript-eslint/no-var-requires - const { version } = require(join(this.rootDir, 'package.json')); + const { version, build } = require(join(this.rootDir, 'package.json')); this.kibanaVersion = version.replace(/\./g, '_'); + this.pkgBuild = build; } public getConfig(serviceName: string): ApmAgentConfig { @@ -122,6 +124,9 @@ export class ApmConfiguration { } private getGitRev() { + if (this.isDistributable) { + return this.pkgBuild.sha; + } try { return execSync('git rev-parse --short HEAD', { encoding: 'utf-8' as BufferEncoding, diff --git a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts index e205421656fa5..c18069f21180b 100644 --- a/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts +++ b/packages/kbn-apm-config-loader/src/utils/get_config_file_paths.test.ts @@ -17,7 +17,7 @@ * under the License. */ -import { resolve } from 'path'; +import { resolve, join } from 'path'; import { getConfigPath } from '@kbn/utils'; import { getConfigurationFilePaths } from './get_config_file_paths'; @@ -25,10 +25,11 @@ describe('getConfigurationFilePaths', () => { const cwd = process.cwd(); it('retrieve the config file paths from the command line arguments', () => { - const argv = ['--config', '/some/path', '-c', '/some-other-path']; + const argv = ['--config', './relative-path', '-c', '/absolute-path']; + expect(getConfigurationFilePaths(argv)).toEqual([ - resolve(cwd, '/some/path'), - resolve(cwd, '/some-other-path'), + resolve(cwd, join('.', 'relative-path')), + '/absolute-path', ]); }); diff --git a/packages/kbn-apm-config-loader/src/utils/read_config.test.ts b/packages/kbn-apm-config-loader/src/utils/read_config.test.ts index 89b73c5d4e26a..7320e5dcbd6ce 100644 --- a/packages/kbn-apm-config-loader/src/utils/read_config.test.ts +++ b/packages/kbn-apm-config-loader/src/utils/read_config.test.ts @@ -20,7 +20,7 @@ import { relative, resolve } from 'path'; import { getConfigFromFiles } from './read_config'; -const fixtureFile = (name: string) => resolve(`${__dirname}/../../__fixtures__/${name}`); +const fixtureFile = (name: string) => resolve(__dirname, '..', '..', '__fixtures__', name); test('reads single yaml from file system and parses to json', () => { const config = getConfigFromFiles([fixtureFile('config.yml')]); @@ -29,13 +29,13 @@ test('reads single yaml from file system and parses to json', () => { }); test('returns a deep object', () => { - const config = getConfigFromFiles([fixtureFile('/config_flat.yml')]); + const config = getConfigFromFiles([fixtureFile('config_flat.yml')]); expect(config).toMatchSnapshot(); }); test('reads and merges multiple yaml files from file system and parses to json', () => { - const config = getConfigFromFiles([fixtureFile('/one.yml'), fixtureFile('/two.yml')]); + const config = getConfigFromFiles([fixtureFile('one.yml'), fixtureFile('two.yml')]); expect(config).toMatchSnapshot(); }); @@ -44,7 +44,7 @@ test('should inject an environment variable value when setting a value with ${EN process.env.KBN_ENV_VAR1 = 'val1'; process.env.KBN_ENV_VAR2 = 'val2'; - const config = getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]); + const config = getConfigFromFiles([fixtureFile('en_var_ref_config.yml')]); delete process.env.KBN_ENV_VAR1; delete process.env.KBN_ENV_VAR2; @@ -54,7 +54,7 @@ test('should inject an environment variable value when setting a value with ${EN test('should throw an exception when referenced environment variable in a config value does not exist', () => { expect(() => - getConfigFromFiles([fixtureFile('/en_var_ref_config.yml')]) + getConfigFromFiles([fixtureFile('en_var_ref_config.yml')]) ).toThrowErrorMatchingSnapshot(); }); @@ -66,14 +66,14 @@ describe('different cwd()', () => { afterAll(() => process.chdir(originalCwd)); test('resolves relative files based on the cwd', () => { - const relativePath = relative(tempCwd, fixtureFile('/one.yml')); + const relativePath = relative(tempCwd, fixtureFile('one.yml')); const config = getConfigFromFiles([relativePath]); expect(config).toMatchSnapshot(); }); test('fails to load relative paths, not found because of the cwd', () => { - const relativePath = relative(resolve(__dirname, '../../'), fixtureFile('/one.yml')); + const relativePath = relative(resolve(__dirname, '..', '..'), fixtureFile('one.yml')); expect(() => getConfigFromFiles([relativePath])).toThrowError(/ENOENT/); }); }); From 7b320f622a9b8ea34799294674eb5375aaac76b3 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 28 Sep 2020 13:19:07 +0200 Subject: [PATCH 12/12] nit on doc --- packages/kbn-apm-config-loader/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kbn-apm-config-loader/README.md b/packages/kbn-apm-config-loader/README.md index e5280cd36bab9..51623dc745f2c 100644 --- a/packages/kbn-apm-config-loader/README.md +++ b/packages/kbn-apm-config-loader/README.md @@ -6,7 +6,7 @@ This module is only meant to be used by the APM instrumentation script (`src/apm to load the required configuration options from the `kibana.yaml` configuration file with default values. -### Why can't just use @kbn-config? +### Why not just use @kbn-config? `@kbn/config` is the recommended way to load and read the kibana configuration file, however in the specific case of APM, we want to only need the minimal dependencies