Add support for the cdn reporting parameter

routes/error-tracker.js

@@ -19,8 +19,8 @@
  */
 
 import { StatusCodes } from 'http-status-codes';
-import extractReportingParams from '../utils/requests/extract-reporting-params.js';
-import LogTarget from '../utils/log-target.js';
+import { extractReportingParams } from '../utils/requests/extract-reporting-params.js';
+import { LoggingTarget } from '../utils/log-target.js';
 import standardizeStackTrace from '../utils/stacktrace/standardize-stack-trace.js';
 import ignoreMessageOrException from '../utils/stacktrace/should-ignore.js';
 import { unminify } from '../utils/stacktrace/unminify.js';
@@ -78,7 +78,7 @@ async function buildEvent(req, reportingParams, logTarget) {
       service: logTarget.serviceName,
       version: logTarget.versionId,
     },
-    message: [normalizedMessage].concat(unminifiedStack).join('\n'),
+    message: [normalizedMessage, ...unminifiedStack].join('\n'),
     context: {
       httpRequest: {
         method: req.method,
@@ -102,7 +102,7 @@ async function handler(req, res) {
   const params = req.body;
   const reportingParams = extractReportingParams(params);
   const { debug, message, version } = reportingParams;
-  const logTarget = new LogTarget(referrer, reportingParams);
+  const logTarget = new LoggingTarget(referrer, reportingParams);
   const { log } = logTarget;
 
   // Reject requests missing essential info.

test/unit/test-log-target.js

@@ -13,7 +13,7 @@
  */
 
 import * as logs from '../../utils/log.js';
-import LogTarget from '../../utils/log-target.js';
+import { LoggingTarget } from '../../utils/log-target.js';
 
 describe('log target', () => {
   let sandbox;
@@ -45,105 +45,141 @@ describe('log target', () => {
 
   describe('log', () => {
     it('returns error log', async () => {
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
 
       expect(logTarget.log).to.equal(logs.errors);
     });
 
     it('returns ads log for inabox', async () => {
       reportingParams.runtime = 'inabox';
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
 
       expect(logTarget.log).to.equal(logs.ads);
     });
 
     it('returns ads log for signing service error', async () => {
       reportingParams.message = 'Error: Signing service error for google';
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
 
       expect(logTarget.log).to.equal(logs.ads);
     });
 
     it('returns user log for asserts', async () => {
       reportingParams.assert = true;
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
 
       expect(logTarget.log).to.equal(logs.users);
     });
 
     it('returns expected log for expected errors', async () => {
       reportingParams.expected = true;
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
 
       expect(logTarget.log).to.equal(logs.expected);
     });
   });
 
   describe('serviceName', () => {
-    describe('for CDN referrers', () => {
+    describe('referrer split', () => {
       [
         'https://cdn.ampproject.org/mywebsite.com/index.html',
         'https://mywebsite-com.cdn.ampproject.org/index.html',
         'https://mywebsite-com.ampproject.net/index.html',
       ].forEach((referrer) => {
-        it(`records "cdn" for ${referrer}`, () => {
-          const logTarget = new LogTarget(referrer, reportingParams);
-          expect(logTarget.serviceName).to.contain('CDN');
+        it(`correctly records "Google Cache" for referrer ${referrer}`, () => {
+          const logTarget = new LoggingTarget(referrer, reportingParams);
+          expect(logTarget.serviceName).to.contain('Google Cache');
+          expect(logTarget.serviceName).to.not.contain('Publisher Origin');
         });
       });
+
+      ['https://mywebsite.com/index.html', 'https://amp.dev/'].forEach(
+        (referrer) => {
+          it(`correctly records "Publisher Origin" for referrer ${referrer}`, () => {
+            const logTarget = new LoggingTarget(referrer, reportingParams);
+            expect(logTarget.serviceName).to.contain('Publisher Origin');
+            expect(logTarget.serviceName).to.not.contain('Google Cache');
+          });
+        }
+      );
     });
 
     describe('for origin referrers', () => {
       const serviceParams = [
-        ['Origin 1%', { version: '00XXXXXXXXXXXXX' }],
-        ['Origin 1%', { version: '03XXXXXXXXXXXXX' }],
-        ['Origin Production', { version: '01XXXXXXXXXXXXX' }],
-        ['Origin Production', { version: '02XXXXXXXXXXXXX' }],
-        ['Origin Nightly', { version: '04XXXXXXXXXXXXX' }],
-        ['Origin Nightly', { version: '05XXXXXXXXXXXXX' }],
-        ['Origin Experiments', { version: '10XXXXXXXXXXXXX' }],
-        ['Origin Experiments', { version: '11XXXXXXXXXXXXX' }],
-        ['Origin Experiments', { version: '12XXXXXXXXXXXXX' }],
-        ['Origin Inabox-Control-A', { version: '20XXXXXXXXXXXXX' }],
-        ['Origin Inabox-Experiment-A', { version: '21XXXXXXXXXXXXX' }],
-        ['Origin Inabox-Control-B', { version: '22XXXXXXXXXXXXX' }],
-        ['Origin Inabox-Experiment-B', { version: '23XXXXXXXXXXXXX' }],
-        ['Origin Inabox-Control-C', { version: '24XXXXXXXXXXXXX' }],
-        ['Origin Inabox-Experiment-C', { version: '25XXXXXXXXXXXXX' }],
-        [
-          'Origin Production (Expected)',
-          {
-            assert: true,
-            version: '01XXXXXXXXXXXXX',
-            expected: true,
-          },
-        ],
-        [
-          'Origin Inabox-Experiment-B (Expected)',
-          {
-            version: '23XXXXXXXXXXXXX',
-            runtime: 'inabox',
-            expected: true,
-          },
-        ],
+        ['1%', '00XXXXXXXXXXXXX'],
+        ['1%', '03XXXXXXXXXXXXX'],
+        ['Production', '01XXXXXXXXXXXXX'],
+        ['Production', '02XXXXXXXXXXXXX'],
+        ['Nightly', '04XXXXXXXXXXXXX'],
+        ['Nightly', '05XXXXXXXXXXXXX'],
+        ['Experiments', '10XXXXXXXXXXXXX'],
+        ['Experiments', '11XXXXXXXXXXXXX'],
+        ['Experiments', '12XXXXXXXXXXXXX'],
+        ['Inabox-Control-A', '20XXXXXXXXXXXXX'],
+        ['Inabox-Experiment-A', '21XXXXXXXXXXXXX'],
+        ['Inabox-Control-B', '22XXXXXXXXXXXXX'],
+        ['Inabox-Experiment-B', '23XXXXXXXXXXXXX'],
+        ['Inabox-Control-C', '24XXXXXXXXXXXXX'],
+        ['Inabox-Experiment-C', '25XXXXXXXXXXXXX'],
       ];
 
-      for (const [expectedName, params] of serviceParams) {
-        it(`correctly constructs "${expectedName}"`, () => {
-          const logTarget = new LogTarget(
+      for (const [expectedName, version] of serviceParams) {
+        it(`correctly constructs service name for "${expectedName} (${version})"`, () => {
+          const logTarget = new LoggingTarget(
             referrer,
-            Object.assign(reportingParams, params)
+            Object.assign(reportingParams, {
+              version,
+              cdn: 'cdn.ampproject.org',
+            })
           );
 
-          expect(logTarget.serviceName).to.equal(expectedName);
+          expect(logTarget.serviceName).to.equal(
+            `${expectedName} > Publisher Origin (cdn.ampproject.org)`
+          );
         });
       }
+
+      it('correctly constructs service name for expected errors', () => {
+        const logTarget = new LoggingTarget(
+          referrer,
+          Object.assign(reportingParams, {
+            assert: true,
+            expected: true,
+            cdn: 'cdn.ampproject.org',
+          })
+        );
+
+        expect(logTarget.serviceName).to.equal(
+          'Production > Publisher Origin (cdn.ampproject.org) > (Expected)'
+        );
+      });
+
+      ['cdn.ampproject.org', 'ampjs.org'].forEach((cdn) => {
+        it(`correctly constructs service name for JS served from ${cdn}`, () => {
+          const logTarget = new LoggingTarget(
+            referrer,
+            Object.assign(reportingParams, { cdn })
+          );
+
+          expect(logTarget.serviceName).to.equal(
+            `Production > Publisher Origin (${cdn})`
+          );
+        });
+      });
+    });
+
+    it('correctly constructs service name for origin pages with unreported JS CDN', () => {
+      const logTarget = new LoggingTarget(referrer, reportingParams);
+
+      expect(logTarget.serviceName).to.equal(
+        'Production > Publisher Origin (CDN not reported)'
+      );
     });
   });
 
   describe('versionId', () => {
     it('returns the release version string', () => {
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.versionId).to.equal('04-03 Stable (0010+2)');
     });
   });
@@ -154,49 +190,49 @@ describe('log target', () => {
     });
 
     it('throttles Stable by a factor of 10', () => {
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1 / 10, 1e-6);
     });
 
     it('does not throttle canary', () => {
       reportingParams.canary = true;
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1, 1e-6);
     });
 
     it('does not throttle Control', () => {
       reportingParams.binaryType = 'control';
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1, 1e-6);
     });
 
     it('does not throttle RC', () => {
       reportingParams.binaryType = 'rc';
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1, 1e-6);
     });
 
     it('does not throttle Nightly', () => {
       reportingParams.binaryType = 'nightly';
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1, 1e-6);
     });
 
     it('throttles user errors by a factor of 10', () => {
       reportingParams.assert = true;
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1 / 100, 1e-6);
     });
 
     it('throttles errors from origin pages by a factor of 10', () => {
       referrer = 'https://myrandomwebsite.com';
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1 / 10, 1e-6);
     });
 
     it('throttles expected errors by a factor of 10', () => {
       reportingParams.expected = true;
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1 / 100, 1e-6);
     });
 
@@ -205,7 +241,7 @@ describe('log target', () => {
       reportingParams.assert = true;
       reportingParams.binaryType = 'rc';
       reportingParams.expected = true;
-      const logTarget = new LogTarget(referrer, reportingParams);
+      const logTarget = new LoggingTarget(referrer, reportingParams);
       expect(logTarget.throttleRate).to.be.closeTo(1 / 100, 1e-6);
     });
   });

utils/log-target.js

@@ -22,14 +22,14 @@ import * as logs from './log.js';
 import humanRtv from './rtv/human-rtv.js';
 import releaseChannels from './rtv/release-channels.js';
 
-const CDN_REGEX = new RegExp(
+const AMP_CACHE_REGEX = new RegExp(
   '^https://cdn\\.ampproject.org/|' +
     '\\.cdn\\.ampproject\\.org/|' +
     '\\.ampproject\\.net/',
   'i'
 );
 
-export default class LoggingTarget {
+export class LoggingTarget {
   constructor(referrer, reportingParams) {
     this.opts = { referrer, ...reportingParams };
     this.log = this.getLog();
@@ -59,23 +59,25 @@ export default class LoggingTarget {
 
   /** Construct the service bucket name for Stackdriver logging. */
   get serviceName() {
-    const { expected, referrer, version } = this.opts;
+    const { cdn, expected, referrer, version } = this.opts;
     const rtvPrefix = version.substr(0, 2);
 
-    const name = [CDN_REGEX.test(referrer) ? 'CDN' : 'Origin'];
-    name.push(
-      rtvPrefix in releaseChannels
-        ? releaseChannels[rtvPrefix].group
-        : 'Unknown'
-    );
+    const name = [releaseChannels[rtvPrefix]?.group ?? '[Unspecified Channel]'];
+    if (AMP_CACHE_REGEX.test(referrer)) {
+      name.push('Google Cache');
+    } else if (cdn) {
+      name.push(`Publisher Origin (${cdn})`);
+    } else {
+      name.push(`Publisher Origin (CDN not reported)`);
+    }
 
     if (expected && this.getLog() !== logs.expected) {
       // Expected errors are split out of the main bucket, but are present for
       // user and inabox errors.
       name.push('(Expected)');
     }
 
-    return name.join(' ');
+    return name.join(' > ');
   }
 
   /** Determine the version identifier to report to Stackdriver logging. */

utils/requests/extract-reporting-params.js

@@ -20,14 +20,15 @@
 import { stringify } from './query-string.js';
 import safeDecodeURIComponent from 'safe-decode-uri-component';
 
-function extractReportingParams(params) {
+export function extractReportingParams(params) {
   const boolProp = (key) => params[key] === '1';
   const strProp = (key) => params[key] || '';
 
   return {
     assert: boolProp('a'),
     binaryType: strProp('bt'),
     canary: boolProp('ca'),
+    cdn: strProp('cdn'),
     debug: boolProp('debug'),
     expected: boolProp('ex'),
     message: safeDecodeURIComponent(strProp('m')),
@@ -40,5 +41,3 @@ function extractReportingParams(params) {
     version: params.v,
   };
 }
-
-export default extractReportingParams;

utils/rtv/latest-rtv.js

@@ -41,9 +41,11 @@ export async function latestRtv() {
     const res = await fetch(url);
     const { ampRuntimeVersion, diversions, ltsRuntimeVersion } =
       await res.json();
-    const versions = [ampRuntimeVersion, ltsRuntimeVersion]
-      .concat(diversions)
-      .filter(Boolean);
+    const versions = [
+      ampRuntimeVersion,
+      ltsRuntimeVersion,
+      ...diversions,
+    ].filter(Boolean);
 
     cache.set(url, versions);
     return versions;