From 15ff0e733dc8c9a954a92fce6ca0461f02dcbd5e Mon Sep 17 00:00:00 2001 From: Josh Dover Date: Wed, 9 Sep 2020 09:45:40 -0600 Subject: [PATCH] PR comments --- src/core/server/metrics/collectors/cgroup.ts | 53 ++++++++++++------- .../metrics/collectors/collector.mock.ts | 33 ++++++++++++ .../metrics/collectors/os.test.mocks.ts | 25 +++++++++ src/core/server/metrics/collectors/os.test.ts | 8 +++ .../server/metrics/metrics_service.mock.ts | 4 +- 5 files changed, 103 insertions(+), 20 deletions(-) create mode 100644 src/core/server/metrics/collectors/collector.mock.ts create mode 100644 src/core/server/metrics/collectors/os.test.mocks.ts diff --git a/src/core/server/metrics/collectors/cgroup.ts b/src/core/server/metrics/collectors/cgroup.ts index 1cae46dbacfaf..867ea44dff1ae 100644 --- a/src/core/server/metrics/collectors/cgroup.ts +++ b/src/core/server/metrics/collectors/cgroup.ts @@ -29,42 +29,35 @@ interface OsCgroupMetricsCollectorOptions { } export class OsCgroupMetricsCollector implements MetricsCollector { - // Used to prevent unnecessary file reads on systems not using cgroups + /** Used to prevent unnecessary file reads on systems not using cgroups. */ private noCgroupPresent = false; + private cpuPath?: string; + private cpuAcctPath?: string; constructor(private readonly options: OsCgroupMetricsCollectorOptions) {} public async collect(): Promise { - if (this.noCgroupPresent) { - return {}; - } - try { - const cgroups = await readControlGroups(); - const cpuPath = this.options.cpuPath || cgroups[GROUP_CPU]; - const cpuAcctPath = this.options.cpuAcctPath || cgroups[GROUP_CPUACCT]; - - // prevents undefined cgroup paths - if (!cpuPath || !cpuAcctPath) { - this.noCgroupPresent = true; + await this.initializePaths(); + if (this.noCgroupPresent || !this.cpuAcctPath || !this.cpuPath) { return {}; } const [cpuAcctUsage, cpuFsPeriod, cpuFsQuota, cpuStat] = await Promise.all([ - readCPUAcctUsage(cpuAcctPath), - readCPUFsPeriod(cpuPath), - readCPUFsQuota(cpuPath), - readCPUStat(cpuPath), + readCPUAcctUsage(this.cpuAcctPath), + readCPUFsPeriod(this.cpuPath), + readCPUFsQuota(this.cpuPath), + readCPUStat(this.cpuPath), ]); return { cpuacct: { - control_group: cpuAcctPath, + control_group: this.cpuAcctPath, usage_nanos: cpuAcctUsage, }, cpu: { - control_group: cpuPath, + control_group: this.cpuPath, cfs_period_micros: cpuFsPeriod, cfs_quota_micros: cpuFsQuota, stat: cpuStat, @@ -72,6 +65,7 @@ export class OsCgroupMetricsCollector implements MetricsCollector> => { + const collector: jest.Mocked> = { + collect: jest.fn().mockResolvedValue(collectReturnValue), + reset: jest.fn(), + }; + + return collector; +}; + +export const metricsCollectorMock = { + create: createCollector, +}; diff --git a/src/core/server/metrics/collectors/os.test.mocks.ts b/src/core/server/metrics/collectors/os.test.mocks.ts new file mode 100644 index 0000000000000..ee02b8c802151 --- /dev/null +++ b/src/core/server/metrics/collectors/os.test.mocks.ts @@ -0,0 +1,25 @@ +/* + * 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 { metricsCollectorMock } from './collector.mock'; + +export const cgroupCollectorMock = metricsCollectorMock.create(); +jest.doMock('./cgroup', () => ({ + OsCgroupMetricsCollector: jest.fn(() => cgroupCollectorMock), +})); diff --git a/src/core/server/metrics/collectors/os.test.ts b/src/core/server/metrics/collectors/os.test.ts index 7d5a6da90b7d6..5e52cecb76be3 100644 --- a/src/core/server/metrics/collectors/os.test.ts +++ b/src/core/server/metrics/collectors/os.test.ts @@ -20,6 +20,7 @@ jest.mock('getos', () => (cb: Function) => cb(null, { dist: 'distrib', release: 'release' })); import os from 'os'; +import { cgroupCollectorMock } from './os.test.mocks'; import { OsMetricsCollector } from './os'; describe('OsMetricsCollector', () => { @@ -27,6 +28,8 @@ describe('OsMetricsCollector', () => { beforeEach(() => { collector = new OsMetricsCollector(); + cgroupCollectorMock.collect.mockReset(); + cgroupCollectorMock.reset.mockReset(); }); afterEach(() => { @@ -96,4 +99,9 @@ describe('OsMetricsCollector', () => { '15m': fifteenMinLoad, }); }); + + it('calls the cgroup sub-collector', async () => { + await collector.collect(); + expect(cgroupCollectorMock.collect).toHaveBeenCalled(); + }); }); diff --git a/src/core/server/metrics/metrics_service.mock.ts b/src/core/server/metrics/metrics_service.mock.ts index 3cd0a322e35e3..2af653004a479 100644 --- a/src/core/server/metrics/metrics_service.mock.ts +++ b/src/core/server/metrics/metrics_service.mock.ts @@ -76,8 +76,8 @@ type MetricsServiceContract = PublicMethodsOf; const createMock = () => { const mocked: jest.Mocked = { - setup: jest.fn().mockReturnValue(createSetupContractMock()), - start: jest.fn().mockReturnValue(createStartContractMock()), + setup: jest.fn().mockReturnValue(createInternalSetupContractMock()), + start: jest.fn().mockReturnValue(createInternalStartContractMock()), stop: jest.fn(), }; return mocked;