feat: Implement cloud logging and error reporting (#391)

* feat: Implement cloud logging and error reporting Part of relaycorp/cloud-gateway#10 * integrate in fastify * move fastify utils into dir * define GATEWAY_VERSION * integrate cogrpc * integrate pdcout * integrate crc queue * convert logLevel to logging.level * capture logging.target * capture logging.envName * configure LOG_LEVEL across all components
relaycorp · Jan 30, 2021 · a68083b · a68083b
1 parent 96a4a6c
commit a68083b
Show file tree

Hide file tree

Showing 35 changed files with 299 additions and 146 deletions.
diff --git a/chart/templates/global-cm.yml b/chart/templates/global-cm.yml
@@ -5,6 +5,16 @@ metadata:
   labels:
     {{- include "relaynet-internet-gateway.labels" . | nindent 4 }}
 data:
+  GATEWAY_VERSION: {{ .Chart.Version }}
+
+  LOG_LEVEL: {{ .Values.logging.level | quote }}
+  {{- if .Values.logging.target }}
+  LOG_TARGET: {{ .Values.logging.target }}
+  {{- end }}
+  {{- if .Values.logging.envName }}
+  LOG_ENV_NAME: {{ .Values.logging.envName }}
+  {{- end }}
+
   NATS_SERVER_URL: {{ .Values.nats.serverUrl }}
   NATS_CLUSTER_ID: {{ .Values.nats.clusterId }}
 

diff --git a/chart/templates/pohttp-deploy.yml b/chart/templates/pohttp-deploy.yml
@@ -41,8 +41,6 @@ spec:
             - name: REQUEST_ID_HEADER
               value: {{ .Values.proxyRequestIdHeader | quote }}
             {{- end }}
-            - name: LOG_LEVEL
-              value: {{ .Values.logLevel | quote }}
           envFrom:
             - configMapRef:
                 name: {{ include "relaynet-internet-gateway.fullname" . }}

diff --git a/chart/templates/poweb-deploy.yml b/chart/templates/poweb-deploy.yml
@@ -42,8 +42,6 @@ spec:
             - name: REQUEST_ID_HEADER
               value: {{ .Values.proxyRequestIdHeader | quote }}
             {{- end }}
-            - name: LOG_LEVEL
-              value: {{ .Values.logLevel | quote }}
           envFrom:
             - configMapRef:
                 name: {{ include "relaynet-internet-gateway.fullname" . }}

diff --git a/chart/values.dev.yml b/chart/values.dev.yml
@@ -8,7 +8,8 @@ fullnameOverride: public-gateway
 tags:
   dev: true
 
-logLevel: debug
+logging:
+  level: debug
 
 ingress:
   enableTls: false

diff --git a/chart/values.schema.json b/chart/values.schema.json
@@ -2,17 +2,30 @@
   "$schema": "http://json-schema.org/schema#",
   "type": "object",
   "required": [
-    "logLevel",
+    "logging",
     "publicAddress",
     "pdcQueue",
     "mongo",
     "nats",
     "vault"
   ],
   "properties": {
-    "logLevel": {
-      "type": "string",
-      "enum": ["debug", "info", "warn", "error"]
+    "logging": {
+      "type": "object",
+      "required": ["level"],
+      "properties": {
+        "level": {
+          "type": "string",
+          "enum": ["debug", "info", "warn", "error", "fatal"]
+        },
+        "target": {
+          "type": "string",
+          "enum": ["gcp"]
+        },
+        "envName": {
+          "type": "string"
+        }
+      }
     },
     "ingress": {
       "type": "object",

diff --git a/chart/values.yaml b/chart/values.yaml
@@ -5,7 +5,8 @@
 tags:
   dev: false
 
-logLevel: info
+logging:
+  level: info
 
 image:
   repository: ghcr.io/relaycorp/relaynet-internet-gateway

diff --git a/docs/install.md b/docs/install.md
@@ -61,6 +61,9 @@ Check out [`relaycorp/cloud-gateway`](https://github.com/relaycorp/cloud-gateway
 | `ingress.annotations` | object | `{}` | Annotations for the ingress |
 | `ingress.enableTls` | boolean | `true` | Whether the ingress should use TLS. You still have to configure TLS through your cloud provider; see [GKE documentation](https://cloud.google.com/kubernetes-engine/docs/concepts/ingress), for example. |
 | `objectStore.backend` | string | | The type of object store used. Any value supported by [`@relaycorp/object-storage`](https://github.com/relaycorp/object-storage-js) (e.g., `s3`). |
+| `logging.level` | string | `info` | The [log level](./instrumentation.md). |
+| `logging.target` | string | | Any target supported by [@relaycorp/pino-cloud](https://www.npmjs.com/package/@relaycorp/pino-cloud); e.g., `gcp`. |
+| `logging.envName` | string | `relaynet-internet-gateway` | A unique name for this instance of the gateway. Used by the `gcp` target as the _service name_ when pushing errors to Google Error Reporting, for example. |
 
 ### Component-specific options
 

diff --git a/docs/instrumentation.md b/docs/instrumentation.md
@@ -4,7 +4,7 @@ permalink: /instrumentation
 ---
 # Instrumentation
 
-We use [pino](https://getpino.io/) to provide structured logs, which could in turn be consumed to provide [instrumentation](https://john-millikin.com/sre-school/instrumentation).
+We use [pino](https://getpino.io/) with [`@relaycorp/pino-cloud`](https://www.npmjs.com/package/@relaycorp/pino-cloud) to provide structured logs, which could in turn be consumed to provide [instrumentation](https://john-millikin.com/sre-school/instrumentation).
 
 ## Common logging attributes
 
@@ -22,4 +22,4 @@ We use log levels as follows:
 - `info`: Events for any outcome observed outside the gateway, or any unusual interaction with a backing service.
 - `warning`: Something has gone wrong but it's being handled gracefully. Triage can start on the next working day.
 - `error`: Something has gone wrong and triage must start within a few minutes. Wake up an SRE if necessary.
-- `critical`: Not used.
+- `fatal`: Not used.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -17,7 +17,7 @@
     "test:ci:unit": "run-s build test:ci:unit:jest",
     "test:ci:unit:jest": "jest --config jest.config.ci.js --coverage",
     "cov": "run-s build test:unit && opn coverage/lcov-report/index.html",
-    "clean": "trash build test"
+    "clean": "trash build test coverage"
   },
   "engines": {
     "node": ">=12"
@@ -70,6 +70,7 @@
     "@relaycorp/cogrpc": "^1.3.1",
     "@relaycorp/keystore-vault": "^1.2.1",
     "@relaycorp/object-storage": "^1.3.2",
+    "@relaycorp/pino-cloud": "^1.0.2",
     "@relaycorp/relaynet-core": "^1.42.3",
     "@relaycorp/relaynet-pohttp": "^1.6.0",
     "@typegoose/typegoose": "^7.4.8",

diff --git a/src/_test_utils.ts b/src/_test_utils.ts
@@ -72,20 +72,6 @@ export function mockSpy<T, Y extends any[]>(
   return spy;
 }
 
-/**
- * @deprecated Use `makeMockLogging()` instead.
- */
-export function mockPino(): pino.Logger {
-  const mockPinoLogger = {
-    debug: mockSpy(jest.fn()),
-    error: mockSpy(jest.fn()),
-    info: mockSpy(jest.fn()),
-    warn: mockSpy(jest.fn()),
-  };
-  jest.mock('pino', () => jest.fn().mockImplementation(() => mockPinoLogger));
-  return mockPinoLogger as any;
-}
-
 // tslint:disable-next-line:readonly-array
 export type MockLogSet = object[];
 

diff --git a/src/bin/pohttp-server.ts b/src/bin/pohttp-server.ts
@@ -1,7 +1,7 @@
 // tslint:disable-next-line:no-var-requires
 require('make-promises-safe');
 
-import { runFastify } from '../services/fastifyUtils';
 import { makeServer } from '../services/pohttp/server';
+import { runFastify } from '../utilities/fastify';
 
 makeServer().then(runFastify);
diff --git a/src/bin/poweb-server.ts b/src/bin/poweb-server.ts
@@ -1,7 +1,7 @@
 // tslint:disable-next-line:no-var-requires
 require('make-promises-safe');
 
-import { runFastify } from '../services/fastifyUtils';
 import { makeServer } from '../services/poweb/server';
+import { runFastify } from '../utilities/fastify';
 
 makeServer().then(runFastify);
diff --git a/src/services/_test_utils.ts b/src/services/_test_utils.ts
@@ -15,7 +15,7 @@ import { Connection } from 'mongoose';
 import * as stan from 'node-nats-streaming';
 
 import { PdaChain } from '../_test_utils';
-import { HTTP_METHODS } from './fastifyUtils';
+import { HTTP_METHODS } from '../utilities/fastify';
 
 export const TOMORROW = new Date();
 TOMORROW.setDate(TOMORROW.getDate() + 1);

diff --git a/src/services/cogrpc/server.spec.ts b/src/services/cogrpc/server.spec.ts
@@ -5,7 +5,8 @@ import { Logger } from 'pino';
 import selfsigned from 'selfsigned';
 
 import { makeMockLogging, mockSpy, partialPinoLog } from '../../_test_utils';
-import { configureMockEnvVars } from '../_test_utils';
+import * as logging from '../../utilities/logging';
+import { configureMockEnvVars, getMockContext } from '../_test_utils';
 import { MAX_RAMF_MESSAGE_SIZE } from '../constants';
 import { runServer } from './server';
 import * as cogrpcService from './service';
@@ -42,6 +43,9 @@ const BASE_ENV_VARS = {
 };
 const mockEnvVars = configureMockEnvVars(BASE_ENV_VARS);
 
+const mockLogger = makeMockLogging().logger;
+const mockMakeLogger = mockSpy(jest.spyOn(logging, 'makeLogger'), () => mockLogger);
+
 describe('runServer', () => {
   test.each([
     'GATEWAY_KEY_ID',
@@ -126,6 +130,16 @@ describe('runServer', () => {
     expect(mockServer.addService).toBeCalledWith(CargoRelayService, serviceImplementation);
   });
 
+  test('Logger should be configured if custom logger is absent', async () => {
+    await runServer();
+
+    expect(mockMakeLogger).toBeCalledWith();
+    const logger = getMockContext(mockMakeLogger).results[0].value;
+    expect(makeServiceImplementationSpy).toBeCalledWith(
+      expect.objectContaining({ baseLogger: logger }),
+    );
+  });
+
   test('Health check service should be added', async () => {
     await runServer();
 

diff --git a/src/services/cogrpc/server.ts b/src/services/cogrpc/server.ts
@@ -2,9 +2,10 @@ import { CargoRelayService } from '@relaycorp/cogrpc';
 import { get as getEnvVar } from 'env-var';
 import { KeyCertPair, Server, ServerCredentials } from 'grpc';
 import grpcHealthCheck from 'grpc-health-check';
-import pino, { Logger } from 'pino';
+import { Logger } from 'pino';
 import * as selfsigned from 'selfsigned';
 
+import { makeLogger } from '../../utilities/logging';
 import { MAX_RAMF_MESSAGE_SIZE } from '../constants';
 import { makeServiceImplementation } from './service';
 
@@ -33,7 +34,7 @@ export async function runServer(logger?: Logger): Promise<void> {
     'grpc.max_receive_message_length': MAX_RECEIVED_MESSAGE_LENGTH,
   });
 
-  const baseLogger = logger ?? pino();
+  const baseLogger = logger ?? makeLogger();
   const serviceImplementation = await makeServiceImplementation({
     baseLogger,
     gatewayKeyIdBase64,