Don't retry flushing CloudWatch metrics

packages/pwa-kit-runtime/CHANGELOG.md

@@ -1,4 +1,5 @@
 ## v2.3.0-dev (Aug 25, 2022)
+- Performance: Skip retries when flushing CloudWatch metrics, prioritize returning a response instead. [720](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/720)
 ## v2.2.0 (Aug 25, 2022)
 ## v2.1.0 (Jul 05, 2022)
 

packages/pwa-kit-runtime/src/utils/ssr-server.test.js

@@ -669,15 +669,8 @@ describe('MetricsSender', () => {
         sender.send(metrics1)
         expect(sender.queueLength).toEqual(metrics1.length)
         return sender.flush().then(() => {
-            const expectedAttempts = sender._retries
-
             // Expect that we retried the correct number of times
-            expect(sender._CW.putMetricData.callCount).toBe(expectedAttempts)
-
-            // Expect that there was a console warning for each retry
-            expect(consoleWarn.callCount).toBe(expectedAttempts)
-
-            // Expect that the MetricsSender is now empty
+            expect(sender._CW.putMetricData.callCount).toBe(1)
             expect(sender.queueLength).toBe(0)
         })
     })

packages/pwa-kit-runtime/src/utils/ssr-server/metrics-sender.js

@@ -4,9 +4,7 @@
  * SPDX-License-Identifier: BSD-3-Clause
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
-import {isRemote, localDevLog} from './utils'
-
-const METRIC_RETRIES = 3
+import {isRemote} from './utils'
 
 /**
  * A class that will handle asynchronous sending of CloudWatch
@@ -27,9 +25,6 @@ export class MetricsSender {
         // name/value metric, and they accumulate on this queue
         // until batched up into a putMetricData call.
         this._queue = []
-
-        // The default retry count
-        this._retries = METRIC_RETRIES
     }
 
     /**
@@ -50,9 +45,6 @@ export class MetricsSender {
     _setup() {
         /* istanbul ignore next */
         if (!this._CW && (isRemote() || MetricsSender._override)) {
-            // Load the AWS sdk. Although we do this on-demand, it's
-            // likely that a webpacked project will include the whole
-            // required module, so we require just the client that we need.
             const Cloudwatch = require('aws-sdk/clients/cloudwatch')
             this._CW = new Cloudwatch({
                 apiVersion: '2010-08-01',
@@ -80,59 +72,18 @@ export class MetricsSender {
         }
 
         return new Promise((resolve) => {
-            // The parameters for putMetricData
-            const params = {
-                MetricData: metrics,
-                Namespace: 'ssr'
-            }
-
-            // Initialize the retry count
-            let retries = this._retries
-
-            // Initialize the retry delay
-            // This delay is long enough to reduce the sending
-            // rate, but not so much that it delays completion
-            // of a response too much.
-            let retryDelay = 50 // mS
-
-            // This callback is called when the putMetricData operation
-            // is complete, or if an error occurs.
-            const callback = (err) => {
-                let retry = false
-
-                if (err) {
-                    if (err.code === 'Throttling') {
-                        // We exceeded the CloudWatch metric sending
-                        // rate. We may need to retry.
-                        retry = --retries > 0
-                        const msg = retry ? `retrying after ${retryDelay} mS` : 'not retrying'
-                        console.warn(
-                            `Metrics: ${err} when sending metric data (length ${metrics.length}): ${msg}`
-                        )
-                    } else {
-                        // Some other error
+            cw.putMetricData(
+                {
+                    MetricData: metrics,
+                    Namespace: 'ssr'
+                },
+                (err) => {
+                    if (err) {
                         console.warn(`Metrics: error sending data: ${err}`)
                     }
-                }
-
-                if (retry) {
-                    // We need to delay before we actually retry
-                    setTimeout(() => cw.putMetricData(params, callback), retryDelay)
-                    // Increase the delay for any future retry
-                    retryDelay += 25 // mS
-                } else {
-                    // Sending is done.
-                    localDevLog(
-                        `Metrics: ${err ? 'discarded' : 'completed'} sending ${
-                            metrics.length
-                        } metric(s)`
-                    )
                     resolve()
                 }
-            }
-
-            // Send the metrics
-            cw.putMetricData(params, callback)
+            )
         })
     }
 

packages/pwa-kit-runtime/src/utils/ssr-server/utils.js

@@ -14,16 +14,14 @@ import crypto from 'crypto'
 import {PROXY_PATH_PREFIX} from '../../ssr/server/constants'
 import {proxyConfigs} from '../ssr-shared'
 
-const BUNDLE_ID = process.env.BUNDLE_ID
-
 // TODO: Clean this up or provide a way to toggle
 export const verboseProxyLogging = false
 
 export const isRemote = () =>
     Object.prototype.hasOwnProperty.call(process.env, 'AWS_LAMBDA_FUNCTION_NAME')
 
 export const getBundleBaseUrl = () => {
-    return `/mobify/bundle/${isRemote() ? BUNDLE_ID : 'development'}/`
+    return `/mobify/bundle/${isRemote() ? process.env.BUNDLE_ID : 'development'}/`
 }
 
 let QUIET = false

packages/pwa-kit-runtime/src/utils/ssr-server/utils.test.js

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2022, Salesforce, Inc.
+ * All rights reserved.
+ * SPDX-License-Identifier: BSD-3-Clause
+ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+import * as utils from './utils'
+
+describe.each([[true], [false]])('Utils remote/local tests (isRemote: %p)', (isRemote) => {
+    let originalEnv
+    const bundleId = 'test-bundle-id-12345'
+
+    beforeEach(() => {
+        originalEnv = process.env
+        process.env = Object.assign({}, process.env)
+        process.env.BUNDLE_ID = bundleId
+        if (isRemote) {
+            process.env.AWS_LAMBDA_FUNCTION_NAME = 'remote-test-name'
+        }
+    })
+
+    afterEach(() => {
+        process.env = originalEnv
+        jest.restoreAllMocks()
+    })
+
+    test(`getBundleBaseUrl should return the correct URL`, () => {
+        const expectedId = isRemote ? bundleId : 'development'
+        const expected = `/mobify/bundle/${expectedId}/`
+        expect(utils.getBundleBaseUrl()).toBe(expected)
+    })
+
+    describe.each([[true], [false]])('Quiet/loud tests', (quiet) => {
+        let originalQuiet
+
+        beforeEach(() => {
+            originalQuiet = utils.isQuiet()
+            utils.setQuiet(quiet)
+        })
+
+        afterEach(() => {
+            utils.setQuiet(originalQuiet)
+            jest.restoreAllMocks()
+        })
+
+        test(`localDevLog should log conditionally (quiet: ${quiet})`, () => {
+            const log = jest.spyOn(console, 'log').mockImplementation(() => {})
+            const msg = 'message'
+            utils.localDevLog(msg)
+            const expected = !isRemote && !quiet ? [[msg]] : []
+            expect(log.mock.calls).toEqual(expected)
+        })
+
+        test(`infoLog should log conditionally (quiet: ${quiet})`, () => {
+            const log = jest.spyOn(console, 'log').mockImplementation(() => {})
+            const msg = 'message'
+            utils.infoLog(msg)
+            const expected = !quiet ? [[msg]] : []
+            expect(log.mock.calls).toEqual(expected)
+        })
+    })
+})