From 90de3c6868c1bd853f435a8fd79e52f3e79e11d8 Mon Sep 17 00:00:00 2001
From: John Knox <jknox@fb.com>
Date: Mon, 14 Jan 2019 06:25:35 -0800
Subject: [PATCH] Add recordSuccessMetric utility

Summary: Wraps a future and records the success rate. This will be stored along with the sessionId and the userId, so we can drill down and get the success rate at different levels.

Reviewed By: passy

Differential Revision: D13635049

fbshipit-source-id: 4e0e6bdc0f9b76b08cf37c5d9fe87909f7338534
---
 src/server.js                       | 22 +++++------
 src/utils/__tests__/metrics.node.js | 58 +++++++++++++++++++++++++++++
 src/utils/metrics.js                | 30 +++++++++++++++
 3 files changed, 98 insertions(+), 12 deletions(-)
 create mode 100644 src/utils/__tests__/metrics.node.js
 create mode 100644 src/utils/metrics.js

diff --git a/src/server.js b/src/server.js
index c96ddcf61..5700aa753 100644
--- a/src/server.js
+++ b/src/server.js
@@ -17,6 +17,7 @@ import {Single} from 'rsocket-flowable';
 import Client from './Client.js';
 import type {UninitializedClient} from './UninitializedClient';
 import {RecurringError} from './utils/errors';
+import {recordSuccessMetric} from './utils/metrics';
 
 const EventEmitter = (require('events'): any);
 const invariant = require('invariant');
@@ -177,8 +178,15 @@ export default class Server extends EventEmitter {
           const {csr, destination} = json;
           return new Single(subscriber => {
             subscriber.onSubscribe();
-            this.certificateProvider
-              .processCertificateSigningRequest(csr, clientData.os, destination)
+            recordSuccessMetric(
+              this.certificateProvider.processCertificateSigningRequest(
+                csr,
+                clientData.os,
+                destination,
+              ),
+              'processCertificateSigningRequest',
+              this.logger,
+            )
               .then(result => {
                 subscriber.onComplete({
                   data: JSON.stringify({
@@ -190,20 +198,10 @@ export default class Server extends EventEmitter {
                   client,
                   deviceId: result.deviceId,
                 });
-                this.logger.track(
-                  'success-rate',
-                  'processCertificateSigningRequest',
-                  1,
-                );
               })
               .catch(e => {
                 subscriber.onError(e);
                 this.emit('client-setup-error', {client, error: e});
-                this.logger.track(
-                  'success-rate',
-                  'processCertificateSigningRequest',
-                  0,
-                );
               });
           });
         }
diff --git a/src/utils/__tests__/metrics.node.js b/src/utils/__tests__/metrics.node.js
new file mode 100644
index 000000000..c2b02e279
--- /dev/null
+++ b/src/utils/__tests__/metrics.node.js
@@ -0,0 +1,58 @@
+/**
+ * Copyright 2018-present Facebook.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ * @format
+ */
+import {recordSuccessMetric} from '../metrics';
+import type Logger from '../../fb-stubs/Logger';
+
+//$FlowFixMe pretend logger is the right type
+const logger: Logger = {
+  track: jest.fn(),
+};
+
+beforeAll(() => {
+  logger.track.mockClear();
+});
+
+test('Wrapping a successful promise preserves result and logs correctly', () => {
+  const successPromise = Promise.resolve('Yay!');
+  const wrappedPromise = recordSuccessMetric(
+    successPromise,
+    'test metric',
+    logger,
+  );
+  return wrappedPromise
+    .then(wrappedValue => {
+      expect(wrappedValue).toBe('Yay!');
+    })
+    .then(() => {
+      expect(logger.track).toHaveBeenCalledWith(
+        'success-rate',
+        'test metric',
+        1,
+      );
+    });
+});
+
+test('Wrapping a rejected promise preserves result and logs correctly', () => {
+  const successPromise = Promise.reject('Oh no!');
+  const wrappedPromise = recordSuccessMetric(
+    successPromise,
+    'test metric',
+    logger,
+  );
+  expect.assertions(2); // Make sure to fail if catch block isn't visited
+  return wrappedPromise
+    .catch(wrappedValue => {
+      expect(wrappedValue).toBe('Oh no!');
+    })
+    .then(() => {
+      expect(logger.track).toHaveBeenCalledWith(
+        'success-rate',
+        'test metric',
+        0,
+      );
+    });
+});
diff --git a/src/utils/metrics.js b/src/utils/metrics.js
new file mode 100644
index 000000000..3816e2faa
--- /dev/null
+++ b/src/utils/metrics.js
@@ -0,0 +1,30 @@
+/**
+ * Copyright 2018-present Facebook.
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ * @format
+ */
+
+import type Logger from '../fb-stubs/Logger';
+
+/*
+ * Wraps a Promise, preserving it's functionality but logging the success or
+ failure state of it, with a given name, based on whether it's fulfilled or
+ rejected.
+ */
+export function recordSuccessMetric(
+  promise: Promise<*>,
+  name: string,
+  logger: Logger,
+): Promise<*> {
+  return promise.then(
+    fulfilledValue => {
+      logger.track('success-rate', name, 1);
+      return fulfilledValue;
+    },
+    rejectionReason => {
+      logger.track('success-rate', name, 0);
+      return Promise.reject(rejectionReason);
+    },
+  );
+}