Skip to content

Commit

Permalink
Stronger typing for monitoring configs (#125467) (#125776)
Browse files Browse the repository at this point in the history
Replaces previous `config.get` pattern typed to `string | undefined` with a full `MonitoringConfig` type.

(cherry picked from commit de206fb)

Co-authored-by: Mat Schaffer <mat@elastic.co>
  • Loading branch information
kibanamachine and matschaffer authored Feb 16, 2022
1 parent 1a3ea1b commit 0335dd6
Show file tree
Hide file tree
Showing 70 changed files with 198 additions and 308 deletions.
29 changes: 8 additions & 21 deletions x-pack/plugins/monitoring/common/ccs_utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import expect from '@kbn/expect';
import sinon from 'sinon';
import { parseCrossClusterPrefix, prefixIndexPattern } from './ccs_utils';

// TODO: tests were not running and are not updated.
Expand All @@ -16,25 +15,20 @@ describe.skip('ccs_utils', () => {
const indexPattern = '.monitoring-xyz-1-*,.monitoring-xyz-2-*';

it('returns the index pattern if ccs is not enabled', () => {
const get = sinon.stub();
const config = { get };

get.withArgs('monitoring.ui.ccs.enabled').returns(false);
// TODO apply as MonitoringConfig during typescript conversion
const config = { ui: { css: { enabled: false } } };

// falsy string values should be ignored
const allPattern = prefixIndexPattern(config, indexPattern, '*');
const onePattern = prefixIndexPattern(config, indexPattern, 'do_not_use_me');

expect(allPattern).to.be(indexPattern);
expect(onePattern).to.be(indexPattern);
expect(get.callCount).to.eql(2);
});

it('returns the index pattern if ccs is not used', () => {
const get = sinon.stub();
const config = { get };

get.withArgs('monitoring.ui.ccs.enabled').returns(true);
// TODO apply as MonitoringConfig during typescript conversion
const config = { ui: { css: { enabled: true } } };

// falsy string values should be ignored
const undefinedPattern = prefixIndexPattern(config, indexPattern);
Expand All @@ -44,14 +38,11 @@ describe.skip('ccs_utils', () => {
expect(undefinedPattern).to.be(indexPattern);
expect(nullPattern).to.be(indexPattern);
expect(blankPattern).to.be(indexPattern);
expect(get.callCount).to.eql(3);
});

it('returns the ccs-prefixed index pattern', () => {
const get = sinon.stub();
const config = { get };

get.withArgs('monitoring.ui.ccs.enabled').returns(true);
// TODO apply as MonitoringConfig during typescript conversion
const config = { ui: { css: { enabled: true } } };

const abcPattern = prefixIndexPattern(config, indexPattern, 'aBc');
const underscorePattern = prefixIndexPattern(config, indexPattern, 'cluster_one');
Expand All @@ -62,14 +53,11 @@ describe.skip('ccs_utils', () => {
expect(underscorePattern).to.eql(
'cluster_one:.monitoring-xyz-1-*,cluster_one:.monitoring-xyz-2-*,cluster_one:monitoring-xyz-1-*,cluster_one:monitoring-xyz-2-*'
);
expect(get.callCount).to.eql(2);
});

it('returns the ccs-prefixed index pattern when wildcard and the local cluster pattern', () => {
const get = sinon.stub();
const config = { get };

get.withArgs('monitoring.ui.ccs.enabled').returns(true);
// TODO apply as MonitoringConfig during typescript conversion
const config = { ui: { css: { enabled: true } } };

const pattern = prefixIndexPattern(config, indexPattern, '*');

Expand All @@ -79,7 +67,6 @@ describe.skip('ccs_utils', () => {
',' +
indexPattern
);
expect(get.callCount).to.eql(1);
});
});

Expand Down
20 changes: 4 additions & 16 deletions x-pack/plugins/monitoring/common/ccs_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,12 @@
* 2.0.
*/

import { isFunction, get } from 'lodash';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import type { MonitoringConfig } from '../server/config';

type Config = Partial<MonitoringConfig> & {
get?: (key: string) => any;
};

export function getConfigCcs(config: Config): boolean {
let ccsEnabled = false;
// TODO: NP
// This function is called with both NP config and LP config
if (isFunction(config.get)) {
ccsEnabled = config.get('monitoring.ui.ccs.enabled');
} else {
ccsEnabled = get(config, 'ui.ccs.enabled');
}
return ccsEnabled;
export function getConfigCcs(config: MonitoringConfig): boolean {
// TODO: (Mat) this function can probably be removed in favor of direct config access where it's used.
return config.ui.ccs.enabled;
}
/**
* Prefix all comma separated index patterns within the original {@code indexPattern}.
Expand All @@ -35,7 +23,7 @@ export function getConfigCcs(config: Config): boolean {
* @param {String} ccs The optional cluster-prefix to prepend.
* @return {String} The index pattern with the {@code cluster} prefix appropriately prepended.
*/
export function prefixIndexPattern(config: Config, indexPattern: string, ccs?: string) {
export function prefixIndexPattern(config: MonitoringConfig, indexPattern: string, ccs?: string) {
const ccsEnabled = getConfigCcs(config);
if (!ccsEnabled || !ccs) {
return indexPattern;
Expand Down
24 changes: 21 additions & 3 deletions x-pack/plugins/monitoring/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
config as ElasticsearchBaseConfig,
ElasticsearchConfig,
} from '../../../../src/core/server/';
import { MonitoringConfigSchema } from './types';

const hostURISchema = schema.uri({ scheme: ['http', 'https'] });

Expand Down Expand Up @@ -79,16 +80,33 @@ export const configSchema = schema.object({
});

export class MonitoringElasticsearchConfig extends ElasticsearchConfig {
public readonly logFetchCount?: number;
public readonly logFetchCount: number;

constructor(rawConfig: TypeOf<typeof monitoringElasticsearchConfigSchema>) {
super(rawConfig as ElasticsearchConfigType);
this.logFetchCount = rawConfig.logFetchCount;
}
}

export type MonitoringConfig = ReturnType<typeof createConfig>;
export function createConfig(config: TypeOf<typeof configSchema>) {
// Build MonitoringConfig type based on MonitoringConfigSchema (config input) but with ui.elasticsearch as a MonitoringElasticsearchConfig (instantiated class)
type MonitoringConfigTypeOverriddenUI = Omit<MonitoringConfigSchema, 'ui'>;

interface MonitoringConfigTypeOverriddenUIElasticsearch
extends Omit<MonitoringConfigSchema['ui'], 'elasticsearch'> {
elasticsearch: MonitoringElasticsearchConfig;
}

/**
* A functional representation of the `monitoring.*` configuration tree passed in from kibana.yml
*/
export interface MonitoringConfig extends MonitoringConfigTypeOverriddenUI {
/**
* A functional representation of the `monitoring.ui.*` configuration tree passed in from kibana.yml
*/
ui: MonitoringConfigTypeOverriddenUIElasticsearch;
}

export function createConfig(config: MonitoringConfigSchema): MonitoringConfig {
return {
...config,
ui: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { getMonitoringUsageCollector } from './get_usage_collector';
import { fetchClusters } from '../../lib/alerts/fetch_clusters';
import { elasticsearchServiceMock } from '../../../../../../src/core/server/mocks';
import { MonitoringConfig } from '../../config';

jest.mock('../../lib/alerts/fetch_clusters', () => ({
fetchClusters: jest.fn().mockImplementation(() => {
Expand Down Expand Up @@ -61,13 +62,13 @@ jest.mock('./lib/fetch_license_type', () => ({
describe('getMonitoringUsageCollector', () => {
const esClient = elasticsearchServiceMock.createClusterClient();
const getEsClient = () => esClient;
const config: any = {
const config = {
ui: {
ccs: {
enabled: true,
},
},
};
} as MonitoringConfig;

it('should be configured correctly', async () => {
const usageCollection: any = {
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/monitoring/server/lib/apm/_apm_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import type { ElasticsearchResponse } from '../../../common/types/es';

const getMemPath = (cgroup?: string) =>
const getMemPath = (cgroup?: boolean) =>
cgroup
? 'beats_stats.metrics.beat.cgroup.memory.mem.usage.bytes'
: 'beats_stats.metrics.beat.memstats.rss';
Expand All @@ -30,7 +30,7 @@ export const apmAggFilterPath = [
'aggregations.max_mem_total.value',
'aggregations.versions.buckets',
];
export const apmUuidsAgg = (maxBucketSize?: string, cgroup?: string) => ({
export const apmUuidsAgg = (maxBucketSize?: number, cgroup?: boolean) => ({
total: {
cardinality: {
field: 'beats_stats.beat.uuid',
Expand Down
7 changes: 4 additions & 3 deletions x-pack/plugins/monitoring/server/lib/apm/get_apm_info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@ import { ApmMetric } from '../metrics';
import { getTimeOfLastEvent } from './_get_time_of_last_event';
import { LegacyRequest } from '../../types';
import { ElasticsearchResponse } from '../../../common/types/es';
import { MonitoringConfig } from '../../config';

export function handleResponse(
response: ElasticsearchResponse,
apmUuid: string,
config: { get: (key: string) => string | undefined }
config: MonitoringConfig
) {
if (!response.hits || response.hits.hits.length === 0) {
return {};
Expand Down Expand Up @@ -68,7 +69,7 @@ export function handleResponse(
eventsDropped: getDiffCalculation(eventsDroppedLast, eventsDroppedFirst),
bytesWritten: getDiffCalculation(Number(bytesWrittenLast), Number(bytesWrittenFirst)),
config: {
container: config.get('monitoring.ui.container.apm.enabled'),
container: config.ui.container.apm.enabled,
},
};
}
Expand Down Expand Up @@ -168,7 +169,7 @@ export async function getApmInfo(
}),
]);

const formattedResponse = handleResponse(response, apmUuid, req.server.config());
const formattedResponse = handleResponse(response, apmUuid, req.server.config);
return {
...formattedResponse,
timeOfLastEvent,
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/monitoring/server/lib/apm/get_apms.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ export function handleResponse(response: ElasticsearchResponse, start: number, e
export async function getApms(req: LegacyRequest, apmIndexPattern: string, clusterUuid: string) {
checkParam(apmIndexPattern, 'apmIndexPattern in getBeats');

const config = req.server.config();
const config = req.server.config;
const start = moment.utc(req.payload.timeRange.min).valueOf();
const end = moment.utc(req.payload.timeRange.max).valueOf();

const params = {
index: apmIndexPattern,
size: config.get('monitoring.ui.max_bucket_size'), // FIXME
size: config.ui.max_bucket_size,
ignore_unavailable: true,
filter_path: [
// only filter path can filter for inner_hits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ export function handleResponse(clusterUuid: string, response: ElasticsearchRespo
export function getApmsForClusters(req: LegacyRequest, clusters: Cluster[], ccs?: string) {
const start = req.payload.timeRange.min;
const end = req.payload.timeRange.max;
const config = req.server.config();
const maxBucketSize = config.get('monitoring.ui.max_bucket_size');
const cgroup = config.get('monitoring.ui.container.apm.enabled');
const config = req.server.config;
const maxBucketSize = config.ui.max_bucket_size;
const cgroup = config.ui.container.apm.enabled;

const indexPatterns = getLegacyIndexPattern({
moduleType: 'beats',
Expand Down Expand Up @@ -82,7 +82,7 @@ export function getApmsForClusters(req: LegacyRequest, clusters: Cluster[], ccs?
return {
...formattedResponse,
config: {
container: config.get('monitoring.ui.container.apm.enabled'),
container: cgroup,
},
stats: {
...formattedResponse.stats,
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/monitoring/server/lib/apm/get_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ export function handleResponse(response: ElasticsearchResponse) {
export async function getStats(req: LegacyRequest, apmIndexPattern: string, clusterUuid: string) {
checkParam(apmIndexPattern, 'apmIndexPattern in getBeats');

const config = req.server.config();
const config = req.server.config;
const start = moment.utc(req.payload.timeRange.min).valueOf();
const end = moment.utc(req.payload.timeRange.max).valueOf();
const maxBucketSize = config.get('monitoring.ui.max_bucket_size');
const cgroup = config.get('monitoring.ui.container.apm.enabled');
const maxBucketSize = config.ui.max_bucket_size;
const cgroup = config.ui.container.apm.enabled;

const params = {
index: apmIndexPattern,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/monitoring/server/lib/beats/_beats_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export const beatsAggFilterPath = [
'aggregations.max_bytes_sent_total.value',
];

export const beatsUuidsAgg = (maxBucketSize: string) => ({
export const beatsUuidsAgg = (maxBucketSize: number) => ({
types: {
terms: {
field: 'beats_stats.beat.type',
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/monitoring/server/lib/beats/get_beats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,13 @@ export function handleResponse(response: ElasticsearchResponse, start: number, e
export async function getBeats(req: LegacyRequest, beatsIndexPattern: string, clusterUuid: string) {
checkParam(beatsIndexPattern, 'beatsIndexPattern in getBeats');

const config = req.server.config();
const config = req.server.config;
const start = moment.utc(req.payload.timeRange.min).valueOf();
const end = moment.utc(req.payload.timeRange.max).valueOf();

const params = {
index: beatsIndexPattern,
size: config.get('monitoring.ui.max_bucket_size'), // FIXME
size: config.ui.max_bucket_size,
ignore_unavailable: true,
filter_path: [
// only filter path can filter for inner_hits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ export function handleResponse(clusterUuid: string, response: ElasticsearchRespo
export function getBeatsForClusters(req: LegacyRequest, clusters: Cluster[], ccs: string) {
const start = req.payload.timeRange.min;
const end = req.payload.timeRange.max;
const config = req.server.config();
const maxBucketSize = config.get('monitoring.ui.max_bucket_size');
const config = req.server.config;
const maxBucketSize = config.ui.max_bucket_size;
const indexPatterns = getLegacyIndexPattern({
moduleType: 'beats',
ccs,
Expand All @@ -57,7 +57,7 @@ export function getBeatsForClusters(req: LegacyRequest, clusters: Cluster[], ccs
clusterUuid,
metric: BeatsClusterMetric.getMetricFields(), // override default of BeatMetric.getMetricFields
}),
aggs: beatsUuidsAgg(maxBucketSize!),
aggs: beatsUuidsAgg(maxBucketSize),
},
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,15 @@ export function handleResponse(response?: BeatsElasticsearchResponse) {
export function getLatestStats(req: LegacyRequest, beatsIndexPattern: string, clusterUuid: string) {
checkParam(beatsIndexPattern, 'beatsIndexPattern in getBeats');

const config = req.server.config();
const config = req.server.config;
const lastDayFilter = { range: { timestamp: { gte: 'now-1d/d', lte: 'now/d' } } };
const beatUuidAgg = {
// size of these buckets determines actual # of beats in each kind of aggregation
aggs: {
uuids: {
terms: {
field: 'beats_stats.beat.uuid',
size: config.get('monitoring.ui.max_bucket_size'),
size: config.ui.max_bucket_size,
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/monitoring/server/lib/beats/get_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ export function handleResponse(response: BeatsElasticsearchResponse) {
export async function getStats(req: LegacyRequest, beatsIndexPattern: string, clusterUuid: string) {
checkParam(beatsIndexPattern, 'beatsIndexPattern in getBeats');

const config = req.server.config();
const config = req.server.config;
const start = moment.utc(req.payload.timeRange.min).valueOf();
const end = moment.utc(req.payload.timeRange.max).valueOf();
const maxBucketSize = config.get('monitoring.ui.max_bucket_size');
const maxBucketSize = config.ui.max_bucket_size;

const params = {
index: beatsIndexPattern,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@ import { flagSupportedClusters } from './flag_supported_clusters';
const mockReq = (log, queryResult = {}) => {
return {
server: {
config() {
return {
get: sinon.stub().withArgs('server.uuid').returns('kibana-1234'),
};
},
instanceUuid: 'kibana-1234',
plugins: {
elasticsearch: {
getCluster() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ async function findSupportedBasicLicenseCluster(
* are also flagged as supported in this method.
*/
export function flagSupportedClusters(req: LegacyRequest, ccs: string) {
const config = req.server.config();
const serverLog = (message: string) => req.getLogger('supported-clusters').debug(message);
const flagAllSupported = (clusters: ElasticsearchModifiedSource[]) => {
clusters.forEach((cluster) => {
Expand Down Expand Up @@ -129,7 +128,7 @@ export function flagSupportedClusters(req: LegacyRequest, ccs: string) {

// if all linked are basic licenses
if (linkedClusterCount === basicLicenseCount) {
const kibanaUuid = config.get('server.uuid') as string;
const kibanaUuid = req.server.instanceUuid;
return await findSupportedBasicLicenseCluster(req, clusters, ccs, kibanaUuid, serverLog);
}

Expand Down
Loading

0 comments on commit 0335dd6

Please sign in to comment.