From 441e9206f49a04f42ff7183a34250cdff00a7fb3 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Fri, 18 Sep 2020 10:45:27 +0200 Subject: [PATCH 01/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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/13] 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 From 21ef5745cd45d65396e621f1f7856b23f579cac2 Mon Sep 17 00:00:00 2001 From: pgayvallet Date: Mon, 28 Sep 2020 14:03:26 +0200 Subject: [PATCH 13/13] Fix --no-basepath references in doc --- docs/developer/advanced/development-basepath.asciidoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer/advanced/development-basepath.asciidoc b/docs/developer/advanced/development-basepath.asciidoc index cb341b9591174..30086288de834 100644 --- a/docs/developer/advanced/development-basepath.asciidoc +++ b/docs/developer/advanced/development-basepath.asciidoc @@ -7,11 +7,11 @@ You can set this explicitly using `server.basePath` in <>. Use the server.rewriteBasePath setting to tell {kib} if it should remove the basePath from requests it receives, and to prevent a deprecation warning at startup. This setting cannot end in a slash (/). -If you want to turn off the basepath when in development mode, start {kib} with the `--no-basepath` flag +If you want to turn off the basepath when in development mode, start {kib} with the `--no-base-path` flag [source,bash] ---- -yarn start --no-basepath +yarn start --no-base-path ----