Repository: GoogleChrome/lighthouse
Pull request #7312 (2019-02-24 11:10:14)

core(config): specify lighthouse channel
<!-- Thank you for submitting a pull request! -->

**Summary**

Determine whether Lighthouse was run through the CLI, npm module, Chrome extension, Chrome DevTools, or Lightrider.

Is settings the right place to put this? Feels like it's similar to `locale`.

**Related Issues/PRs**
<!-- Provide any additional information we might need to understand the pull request -->

Part of https://github.com/GoogleChrome/lighthouse/issues/4663

patrickhulce: > Is it still useful information to know the original source, or should it be `utm_medium=viewer` at that point? Especially if it's shared with multiple other people...none of them are touching the original source in that case.

I think the original source is far more important for environment info than where they are currently looking at it. Maybe not the correct thing from utm medium perspective, but I think viewer is the only real case where there will be a mismatch and maybe viewer just overrides that with UI features or something?
mattzeunert: Any more feedback on this?
mattzeunert: > I don't really think I'm satisfied with the field just being freeform string, I'd prefer a enumerated type on ts and on the proto. I like the longer names, but personally I want them enumerated.

@exterkamp We allow CLI and node module consumers to pass in their own `channel` values. Does it still make sense to enumerate the possible values, since I think we need `string` anyway?

I feel it's similar on the Lightrider/proto side, but maybe we can put `psi` and `webdotdev` in the enum?
exterkamp: Hm, I might have missed this discussion (I don't see it in this PR?), but why do they get to pass their own channel?  Wouldn't their channel be "cli" or "node-module"?  Since that is the channel they were on?  What's the use case for sending a custom run channel that we don't know about beforehand and have enumerated?
brendankenny: I think it comes down to what the purpose of the field is.

As helpful debug info after the fact, it makes sense to stick to just our "clients" (cli/node/devtools/extension/lr). That's the programmatic entry point, great.

However, #4663 was about getting a sense of what lead users to our docs. In that case it makes some sense to have it open so we can distinguish between e.g. the cli and PSI and WPT, which are all very different user experience entry points into our docs.
exterkamp: I've been convinced irl.  sgtm!
mattzeunert: @exterkamp Should be good to merge now 🙂
mattzeunert: @brendankenny Thanks for those suggestions, added them now!
brendankenny: thanks!

Patch:
diff --git a/clients/devtools-entry.js b/clients/devtools-entry.js
index cd542f7b3..ac5b7df9d 100644
--- a/clients/devtools-entry.js
+++ b/clients/devtools-entry.js
@@ -36,6 +36,7 @@ function getDefaultConfigForCategories(categoryIDs) {
 function runLighthouseInWorker(port, url, flags, categoryIDs) {
   // Default to 'info' logging level.
   flags.logLevel = flags.logLevel || 'info';
+  flags.channel = 'devtools';
   const config = getDefaultConfigForCategories(categoryIDs);
   const connection = new RawProtocol(port);
 
diff --git a/clients/extension/scripts/extension-entry.js b/clients/extension/scripts/extension-entry.js
index 84a4a956f..3923af911 100644
--- a/clients/extension/scripts/extension-entry.js
+++ b/clients/extension/scripts/extension-entry.js
@@ -82,6 +82,7 @@ async function runLighthouseInExtension(flags, categoryIDs) {
   // Default to 'info' logging level.
   flags.logLevel = flags.logLevel || 'info';
   flags.output = 'html';
+  flags.channel = 'extension';
 
   const connection = new ExtensionProtocol();
   const url = await connection.getCurrentTabURL();
diff --git a/clients/lightrider-entry.js b/clients/lightrider-entry.js
index 98d1dbc30..a942ac292 100644
--- a/clients/lightrider-entry.js
+++ b/clients/lightrider-entry.js
@@ -36,6 +36,7 @@ async function runLighthouseInLR(connection, url, flags, lrOpts) {
   // disableStorageReset because it causes render server hang
   flags.disableStorageReset = true;
   flags.logLevel = flags.logLevel || 'info';
+  flags.channel = 'lr';
 
   const config = lrDevice === 'desktop' ? LR_PRESETS.desktop : LR_PRESETS.mobile;
   if (categoryIDs) {
diff --git a/lighthouse-cli/cli-flags.js b/lighthouse-cli/cli-flags.js
index a808cfd39..83ecfb7de 100644
--- a/lighthouse-cli/cli-flags.js
+++ b/lighthouse-cli/cli-flags.js
@@ -137,6 +137,7 @@ function getFlags(manualArgv) {
       .array('skipAudits')
       .array('output')
       .string('extraHeaders')
+      .string('channel')
       .string('precomputedLanternDataPath')
       .string('lanternDataOutputPath')
 
@@ -146,6 +147,7 @@ function getFlags(manualArgv) {
       .default('port', 0)
       .default('hostname', 'localhost')
       .default('enable-error-reporting', undefined) // Undefined so prompted by default
+      .default('channel', 'cli')
       .check(/** @param {LH.CliFlags} argv */ (argv) => {
         // Lighthouse doesn't need a URL if...
         //   - We're just listing the available options.
diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js
index 2a050b12b..dad5f907f 100644
--- a/lighthouse-core/config/constants.js
+++ b/lighthouse-core/config/constants.js
@@ -52,6 +52,7 @@ const defaultSettings = {
   disableStorageReset: false,
   disableDeviceEmulation: false,
   emulatedFormFactor: 'mobile',
+  channel: 'node',
 
   // the following settings have no defaults but we still want ensure that `key in settings`
   // in config will work in a typechecked way
diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto
index fcc45bcb1..ce8ba5956 100644
--- a/proto/lighthouse-result.proto
+++ b/proto/lighthouse-result.proto
@@ -139,6 +139,9 @@ message LighthouseResult {
     // List of the categories that were run, empty if all were run
     // nullable list of strings
     google.protobuf.Value only_categories = 3;
+
+    // How Lighthouse was run, e.g. from the Chrome extension or from the npm module
+    string channel = 4;
   }
 
   // The settings that were used to run this audit
diff --git a/types/externs.d.ts b/types/externs.d.ts
index d4d5e5192..e765364ae 100644
--- a/types/externs.d.ts
+++ b/types/externs.d.ts
@@ -123,6 +123,8 @@ declare global {
       skipAudits?: string[] | null;
       /** List of extra HTTP Headers to include. */
       extraHeaders?: Crdp.Network.Headers | null; // See extraHeaders TODO in bin.js
+      /** How Lighthouse was run, e.g. from the Chrome extension or from the npm module */
+      channel?: string
       /** Precomputed lantern estimates to use instead of observed analysis. */
       precomputedLanternData?: PrecomputedLanternData | null;
     }


Test patch:
diff --git a/clients/test/extension/extension-test.js b/clients/test/extension/extension-test.js
index 00bae1276..8f8abed0e 100644
--- a/clients/test/extension/extension-test.js
+++ b/clients/test/extension/extension-test.js
@@ -27,6 +27,7 @@ describe('Lighthouse chrome extension', function() {
   let browser;
   let extensionPage;
   let originalManifest;
+  let lhr;
 
   function getAuditElementsIds({category, selector}) {
     return extensionPage.evaluate(
@@ -102,6 +103,8 @@ describe('Lighthouse chrome extension', function() {
       throw new Error(lighthouseResult.exceptionDetails.text);
     }
 
+    lhr = lighthouseResult.result.value.lhr;
+
     extensionPage = (await browser.pages()).find(page =>
       page.url().includes('blob:chrome-extension://')
     );
@@ -176,4 +179,8 @@ describe('Lighthouse chrome extension', function() {
     // this audit has regressed in the extension twice, so make sure it passes
     assert.ok(await extensionPage.$('#is-crawlable.lh-audit--pass'), 'did not pass is-crawlable');
   });
+
+  it('should specify the channel as extension', async () => {
+    assert.equal(lhr.configSettings.channel, 'extension');
+  });
 });
diff --git a/clients/test/lightrider-entry-test.js b/clients/test/lightrider-entry-test.js
index 2151241c4..90ef0fe5c 100644
--- a/clients/test/lightrider-entry-test.js
+++ b/clients/test/lightrider-entry-test.js
@@ -6,7 +6,9 @@
 'use strict';
 
 const assert = require('assert');
+const jest = require('jest-mock');
 const lhBackground = require('../lightrider-entry.js');
+const Runner = require('../../lighthouse-core/runner.js');
 const LHError = require('../../lighthouse-core/lib/lh-error.js');
 
 /* eslint-env mocha */
@@ -56,5 +58,18 @@ describe('lightrider-entry', () => {
       assert.strictEqual(parsedResult.runtimeError.code, LHError.UNKNOWN_ERROR);
       assert.ok(parsedResult.runtimeError.message.includes(errorMsg));
     });
+
+    it('specifies the channel as lr', async () => {
+      const runStub = jest.spyOn(Runner, 'run');
+
+      const mockConnection = {};
+      const url = 'https://example.com';
+
+      await lhBackground.runLighthouseInLR(mockConnection, url, {}, {});
+      const config = runStub.mock.calls[0][1].config;
+      assert.equal(config.settings.channel, 'lr');
+
+      runStub.mockRestore();
+    });
   });
 });
diff --git a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
index 111adf17a..cd5205cad 100644
--- a/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
+++ b/lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
@@ -1202,6 +1202,7 @@ Object {
     "additionalTraceCategories": null,
     "auditMode": false,
     "blockedUrlPatterns": null,
+    "channel": "cli",
     "disableDeviceEmulation": false,
     "disableStorageReset": false,
     "emulatedFormFactor": "mobile",
@@ -1331,6 +1332,7 @@ Object {
     "additionalTraceCategories": null,
     "auditMode": true,
     "blockedUrlPatterns": null,
+    "channel": "cli",
     "disableDeviceEmulation": false,
     "disableStorageReset": false,
     "emulatedFormFactor": "mobile",
diff --git a/lighthouse-cli/test/cli/run-test.js b/lighthouse-cli/test/cli/run-test.js
index f1681bca6..56ec901e2 100644
--- a/lighthouse-cli/test/cli/run-test.js
+++ b/lighthouse-cli/test/cli/run-test.js
@@ -50,6 +50,8 @@ describe('CLI run', function() {
       assert.deepStrictEqual(results.timing, lhr.timing);
       assert.ok(results.timing.total !== 0);
 
+      assert.equal(results.configSettings.channel, 'cli');
+
       fs.unlinkSync(filename);
     });
   }, 20 * 1000);
diff --git a/lighthouse-core/test/index-test.js b/lighthouse-core/test/index-test.js
index d9a3adc97..bc0c40f5d 100644
--- a/lighthouse-core/test/index-test.js
+++ b/lighthouse-core/test/index-test.js
@@ -132,6 +132,29 @@ describe('Module Tests', function() {
     });
   });
 
+  it('should specify the channel as node by default', async function() {
+    const exampleUrl = 'https://www.reddit.com/r/nba';
+    const results = await lighthouse(exampleUrl, {}, {
+      settings: {
+        auditMode: __dirname + '/fixtures/artifacts/perflog/',
+      },
+      audits: [],
+    });
+    assert.equal(results.lhr.configSettings.channel, 'node');
+  });
+
+  it('lets consumers pass in a custom channel', async function() {
+    const exampleUrl = 'https://www.reddit.com/r/nba';
+    const results = await lighthouse(exampleUrl, {}, {
+      settings: {
+        auditMode: __dirname + '/fixtures/artifacts/perflog/',
+        channel: 'custom',
+      },
+      audits: [],
+    });
+    assert.equal(results.lhr.configSettings.channel, 'custom');
+  });
+
   it('should return a list of audits', function() {
     assert.ok(Array.isArray(lighthouse.getAuditList()));
   });
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
index 04ad0dce0..746d3435c 100644
--- a/lighthouse-core/test/results/sample_v2.json
+++ b/lighthouse-core/test/results/sample_v2.json
@@ -3175,6 +3175,7 @@
     "disableStorageReset": false,
     "disableDeviceEmulation": false,
     "emulatedFormFactor": "mobile",
+    "channel": "cli",
     "locale": "en-US",
     "blockedUrlPatterns": null,
     "additionalTraceCategories": null,