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

Stronger typing for monitoring configs #125467

Merged
merged 13 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
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
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