Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the @kbn/apm-config-loader package #77855

Merged
merged 17 commits into from
Sep 28, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
13 changes: 13 additions & 0 deletions packages/kbn-apm-config-loader/README.md
Original file line number Diff line number Diff line change
@@ -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?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
### 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
before loading `elastic-apm-node` to avoid loosing instrumentation on the already loaded modules.
pgayvallet marked this conversation as resolved.
Show resolved Hide resolved
23 changes: 23 additions & 0 deletions packages/kbn-apm-config-loader/package.json
Original file line number Diff line number Diff line change
@@ -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"
Comment on lines +14 to +17
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To determine the default path of the config file, and of the data folder, I need to import @kbn/utils to use getDataPath() and getConfigPath().

Even with deep imports from @kbn-utils/target/path the module imports @kbn/config-schema, which imports joi

import { TypeOf, schema } from '@kbn/config-schema';
import { REPO_ROOT } from '../repo_root';

Not sure if this is blocker or not. I can split packages/kbn-utils/src/path/index.ts into multiple files to be able to import the helper methods I need without import the schema that is currently in the same file. That would address this problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok if we end up importing Joi in this first pass. We already know it's a source of some slowness and I believe we'll still be able to see that at some level in our traces.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Node.js agent currently doesn't instrument joi, but if we were to do that in the future this could present a problem. But I wouldn't worry about this currently - just thought I'd mention it.

},
"devDependencies": {
"typescript": "4.0.2",
"tsd": "^0.7.4"
}
}
116 changes: 116 additions & 0 deletions packages/kbn-apm-config-loader/src/config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*
* 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, get } 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 {
private baseConfig?: any;
private kibanaVersion: string;

constructor(
private readonly rootDir: string,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can get this from ROOT_DIR in @kbn/utils

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tylersmalley Just thought about something after our zoom meeting: If you were planning to move the src/apm script to @kbn/utils and import @kbn/apm-config-loader from here, We are going to cause a cyclic dependency, as @kbn/apm-config-loader actually also got a dependency to @kbn/utils.

Maybe the src/apm script itself should be moved to @kbn/apm-config-loader instead to avoid this? In that case, I could rename it to something like @kbn/apm-agent for example. Would not change much for future consumers of the agent or agent config. WDYT?

private readonly rawKibanaConfig: Record<string, any>
) {
// 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) {
return {
...this.getBaseConfig(),
serviceName: `${serviceName}-${this.kibanaVersion}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is how it works in the current implementation, but I think we should be using the serviceVersion property rather than appending the Kibana version to the serviceName. I also wonder if it would be useful to include the git revision, something like:

Suggested change
serviceName: `${serviceName}-${this.kibanaVersion}`,
serviceName,
serviceVersion: `${this.kibanaVersion}-${this.git_rev}`

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems better yea, but I'm not sure who's in charge of confirming that the change would be alright? Is that on us?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of you edit,

I think we should merge this without any changes to the APM config right now

I will keep this for a follow up to avoid changing anything from the apm config in the current PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the reason why I originally went with this approach was because the APM UI didn't have a way to filter on serviceVersion - so it would bundle up all traces for Kibana v8.0.0, v7.9.2 etc etc. into the same view. I'm not sure if it's possible to apply such a filter easily today that will stick between clicks to different pages?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the service version filter does stick around in the APM UI when switching between different views now. It'd be nice to use the same serviceName for all versions so that we can do comparisons more easily

};
}

private getBaseConfig() {
if (!this.baseConfig) {
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;
}
this.baseConfig = apmConfig;
}

return this.baseConfig;
}

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 (get(this.rawKibanaConfig, 'server.uuid')) {
return this.rawKibanaConfig.server.uuid;
}

const dataPath: string = get(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 {};
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing this function would be replaced when we add config keys for APM to the regular yaml file to read from this.rawKibanaConfig as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how exactly the apm team was using this, but if it's really just for local development, I guess that once the properties are exposed / consumed from the kibana.yaml config file, the dev config can just go away and be replaced by local kibana.yaml config overrides? Need confirmation from apm though.


private getGitRev() {
try {
return execSync('git rev-parse --short HEAD', {
encoding: 'utf-8' as BufferEncoding,
stdio: ['ignore', 'pipe', 'ignore'],
}).trim();
} catch (e) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for distributables we should fallback to the build sha located on pkg.build.sha

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in 8fc031d

}
}
}
35 changes: 35 additions & 0 deletions packages/kbn-apm-config-loader/src/config_loader.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* 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, applyConfigOverrides } from './utils';

import { ApmConfiguration } from './config';

/**
* Load the APM configuration.
*
* @param argv the `process.argv` arguments
* @param rootDir The root directory of kibana (where the sources and the `package.json` file are)
*/
export const loadConfiguration = (argv: string[], rootDir: string): ApmConfiguration => {
const configPaths = getConfigurationFilePaths(argv);
const rawConfiguration = getConfigFromFiles(configPaths);
applyConfigOverrides(rawConfiguration, argv);
return new ApmConfiguration(rootDir, rawConfiguration);
};
21 changes: 21 additions & 0 deletions packages/kbn-apm-config-loader/src/index.ts
Original file line number Diff line number Diff line change
@@ -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';
38 changes: 38 additions & 0 deletions packages/kbn-apm-config-loader/src/utils/apply_config_overrides.ts
Original file line number Diff line number Diff line change
@@ -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<string, any>, 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);
}
};
61 changes: 61 additions & 0 deletions packages/kbn-apm-config-loader/src/utils/ensure_deep_object.ts
Original file line number Diff line number Diff line change
@@ -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));
}
37 changes: 37 additions & 0 deletions packages/kbn-apm-config-loader/src/utils/get_config_file_paths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* 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';
import { getFlagValues } from './read_argv';

/**
* 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 = (argv: string[]): string[] => {
const rawPaths = getFlagValues(argv, ['-c', '--config']);
if (rawPaths.length) {
return rawPaths.map((path) => resolve(process.cwd(), path));
}
return [getConfigPath()];
};
22 changes: 22 additions & 0 deletions packages/kbn-apm-config-loader/src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* 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';
export { applyConfigOverrides } from './apply_config_overrides';
Loading