Add more robust error handling to OsCgroupMetricsCollector (elastic#7…

…8213)

Co-authored-by: Elastic Machine <[email protected]>

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

@@ -18,6 +18,7 @@
  */
 
 import mockFs from 'mock-fs';
+import { loggerMock } from '@kbn/logging/target/mocks';
 import { OsCgroupMetricsCollector } from './cgroup';
 
 describe('OsCgroupMetricsCollector', () => {
@@ -30,8 +31,10 @@ describe('OsCgroupMetricsCollector', () => {
       },
     });
 
-    const collector = new OsCgroupMetricsCollector({});
+    const logger = loggerMock.create();
+    const collector = new OsCgroupMetricsCollector({ logger });
     expect(await collector.collect()).toEqual({});
+    expect(logger.error).not.toHaveBeenCalled();
   });
 
   it('collects default cgroup data', async () => {
@@ -51,7 +54,7 @@ throttled_time 666
       `,
     });
 
-    const collector = new OsCgroupMetricsCollector({});
+    const collector = new OsCgroupMetricsCollector({ logger: loggerMock.create() });
     expect(await collector.collect()).toMatchInlineSnapshot(`
       Object {
         "cpu": Object {
@@ -90,6 +93,7 @@ throttled_time 666
     });
 
     const collector = new OsCgroupMetricsCollector({
+      logger: loggerMock.create(),
       cpuAcctPath: 'xxcustomcpuacctxx',
       cpuPath: 'xxcustomcpuxx',
     });
@@ -112,4 +116,23 @@ throttled_time 666
       }
     `);
   });
+
+  it('returns empty object and logs error on an EACCES error', async () => {
+    mockFs({
+      '/proc/self/cgroup': `
+123:memory:/groupname
+123:cpu:/groupname
+123:cpuacct:/groupname
+      `,
+      '/sys/fs/cgroup': mockFs.directory({ mode: parseInt('0000', 8) }),
+    });
+
+    const logger = loggerMock.create();
+
+    const collector = new OsCgroupMetricsCollector({ logger });
+    expect(await collector.collect()).toEqual({});
+    expect(logger.error).toHaveBeenCalledWith(
+      "cgroup metrics could not be read due to error: [Error: EACCES, permission denied '/sys/fs/cgroup/cpuacct/groupname/cpuacct.usage']"
+    );
+  });
 });

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

@@ -19,11 +19,13 @@
 
 import fs from 'fs';
 import { join as joinPath } from 'path';
+import { Logger } from '@kbn/logging';
 import { MetricsCollector, OpsOsMetrics } from './types';
 
 type OsCgroupMetrics = Pick<OpsOsMetrics, 'cpu' | 'cpuacct'>;
 
 interface OsCgroupMetricsCollectorOptions {
+  logger: Logger;
   cpuPath?: string;
   cpuAcctPath?: string;
 }
@@ -38,8 +40,12 @@ export class OsCgroupMetricsCollector implements MetricsCollector<OsCgroupMetric
 
   public async collect(): Promise<OsCgroupMetrics> {
     try {
+      if (this.noCgroupPresent) {
+        return {};
+      }
+
       await this.initializePaths();
-      if (this.noCgroupPresent || !this.cpuAcctPath || !this.cpuPath) {
+      if (!this.cpuAcctPath || !this.cpuPath) {
         return {};
       }
 
@@ -64,12 +70,15 @@ export class OsCgroupMetricsCollector implements MetricsCollector<OsCgroupMetric
         },
       };
     } catch (err) {
-      if (err.code === 'ENOENT') {
-        this.noCgroupPresent = true;
-        return {};
-      } else {
-        throw err;
+      this.noCgroupPresent = true;
+
+      if (err.code !== 'ENOENT') {
+        this.options.logger.error(
+          `cgroup metrics could not be read due to error: [${err.toString()}]`
+        );
       }
+
+      return {};
     }
   }
 

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

@@ -19,6 +19,7 @@
 
 jest.mock('getos', () => (cb: Function) => cb(null, { dist: 'distrib', release: 'release' }));
 
+import { loggerMock } from '@kbn/logging/target/mocks';
 import os from 'os';
 import { cgroupCollectorMock } from './os.test.mocks';
 import { OsMetricsCollector } from './os';
@@ -27,7 +28,7 @@ describe('OsMetricsCollector', () => {
   let collector: OsMetricsCollector;
 
   beforeEach(() => {
-    collector = new OsMetricsCollector();
+    collector = new OsMetricsCollector({ logger: loggerMock.create() });
     cgroupCollectorMock.collect.mockReset();
     cgroupCollectorMock.reset.mockReset();
   });

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

@@ -20,21 +20,26 @@
 import os from 'os';
 import getosAsync, { LinuxOs } from 'getos';
 import { promisify } from 'util';
+import { Logger } from '@kbn/logging';
 import { OpsOsMetrics, MetricsCollector } from './types';
 import { OsCgroupMetricsCollector } from './cgroup';
 
 const getos = promisify(getosAsync);
 
 export interface OpsMetricsCollectorOptions {
+  logger: Logger;
   cpuPath?: string;
   cpuAcctPath?: string;
 }
 
 export class OsMetricsCollector implements MetricsCollector<OpsOsMetrics> {
   private readonly cgroupCollector: OsCgroupMetricsCollector;
 
-  constructor(options: OpsMetricsCollectorOptions = {}) {
-    this.cgroupCollector = new OsCgroupMetricsCollector(options);
+  constructor(options: OpsMetricsCollectorOptions) {
+    this.cgroupCollector = new OsCgroupMetricsCollector({
+      ...options,
+      logger: options.logger.get('cgroup'),
+    });
   }
 
   public async collect(): Promise<OpsOsMetrics> {

src/core/server/metrics/metrics_service.ts

@@ -50,7 +50,10 @@ export class MetricsService
       .pipe(first())
       .toPromise();
 
-    this.metricsCollector = new OpsMetricsCollector(http.server, config.cGroupOverrides);
+    this.metricsCollector = new OpsMetricsCollector(http.server, {
+      logger: this.logger,
+      ...config.cGroupOverrides,
+    });
 
     await this.refreshMetrics();
 

src/core/server/metrics/ops_metrics_collector.test.ts

@@ -17,6 +17,7 @@
  * under the License.
  */
 
+import { loggerMock } from '@kbn/logging/target/mocks';
 import {
   mockOsCollector,
   mockProcessCollector,
@@ -30,7 +31,7 @@ describe('OpsMetricsCollector', () => {
 
   beforeEach(() => {
     const hapiServer = httpServiceMock.createInternalSetupContract().server;
-    collector = new OpsMetricsCollector(hapiServer, {});
+    collector = new OpsMetricsCollector(hapiServer, { logger: loggerMock.create() });
 
     mockOsCollector.collect.mockResolvedValue('osMetrics');
   });