PR comments

src/core/server/metrics/collectors/cgroup.ts

@@ -29,49 +29,43 @@ interface OsCgroupMetricsCollectorOptions {
 }
 
 export class OsCgroupMetricsCollector implements MetricsCollector<OsCgroupMetrics> {
- // 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<OsCgroupMetrics> {
- 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,
  },
  };
  } catch (err) {
  if (err.code === 'ENOENT') {
+ this.noCgroupPresent = true;
  return {};
  } else {
  throw err;
@@ -80,6 +74,29 @@ export class OsCgroupMetricsCollector implements MetricsCollector<OsCgroupMetric
  }
 
  public reset() {}
+
+ private async initializePaths() {
+ // Perform this setup lazily on the first collect call and then memoize the results.
+ // Makes the assumption this data doesn't change while the process is running.
+ if (this.cpuPath && this.cpuAcctPath) {
+ return;
+ }
+
+ // Only read the file if both options are undefined.
+ if (!this.options.cpuPath || !this.options.cpuAcctPath) {
+ const cgroups = await readControlGroups();
+ this.cpuPath = this.options.cpuPath || cgroups[GROUP_CPU];
+ this.cpuAcctPath = this.options.cpuAcctPath || cgroups[GROUP_CPUACCT];
+ } else {
+ this.cpuPath = this.options.cpuPath;
+ this.cpuAcctPath = this.options.cpuAcctPath;
+ }
+
+ // prevents undefined cgroup paths
+ if (!this.cpuPath || !this.cpuAcctPath) {
+ this.noCgroupPresent = true;
+ }
+ }
 }
 
 const CONTROL_GROUP_RE = new RegExp('\\d+:([^:]+):(/.*)');

src/core/server/metrics/collectors/collector.mock.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 { MetricsCollector } from './types';
+
+const createCollector = (collectReturnValue: any = {}): jest.Mocked<MetricsCollector<any>> => {
+ const collector: jest.Mocked<MetricsCollector<any>> = {
+ collect: jest.fn().mockResolvedValue(collectReturnValue),
+ reset: jest.fn(),
+ };
+
+ return collector;
+};
+
+export const metricsCollectorMock = {
+ create: createCollector,
+};

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),
+}));

src/core/server/metrics/collectors/os.test.ts

@@ -20,13 +20,16 @@
 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', () => {
  let collector: 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();
+ });
 });

src/core/server/metrics/metrics_service.mock.ts

@@ -76,8 +76,8 @@ type MetricsServiceContract = PublicMethodsOf<MetricsService>;
 
 const createMock = () => {
  const mocked: jest.Mocked<MetricsServiceContract> = {
- 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;