refactor: keep SQL out of the beacon's browser bundle

scripts/sh/pack-check.sh

@@ -150,6 +150,47 @@ if [[ -n "$forbidden" ]]; then
   exit 1
 fi
 
+# The beacon envelope, which has to stand on its own.
+#
+# `beaconQueryString` is the one thing in this package that runs on every page
+# of a measured site. A bundler reaches it through the package root, and the
+# root re-exports everything, so what actually decides the page weight is
+# which modules the envelope itself pulls in. Its module answers that with
+# nothing, and this is what keeps the answer that way.
+#
+# KensioSoftware/rainlytics#110 is what this is guarding. The SQL reading the
+# same parameters back used to sit in the same file, and a minified bundle of
+# a beacon-shaped entry carried `url_decode`, `url_extract_parameter` and
+# `strpos` that no browser runs. Splitting the file took that bundle from 464
+# bytes to 241.
+#
+# The forbidden-import check above cannot see this. The root legitimately
+# reaches SQL, and every one of those imports is a relative path it allows.
+
+envelope="$tmp/package/dist/beacon-events.js"
+
+if [[ ! -f "$envelope" ]]; then
+  echo "The beacon envelope is not at dist/beacon-events.js." >&2
+  echo "Point this check at wherever beaconQueryString moved to." >&2
+  exit 1
+fi
+
+envelope_imports="$(
+  grep --only-matching --extended-regexp \
+    "(from|import)[[:space:]]*\(?[[:space:]]*[\"'][^\"']+[\"']" \
+    "$envelope" || true
+)"
+
+if [[ -n "$envelope_imports" ]]; then
+  echo "The beacon envelope imports something:" >&2
+  sed 's/^/  /' <<<"$envelope_imports" >&2
+  echo >&2
+  echo "Every page of a measured site downloads this module. Whatever it" >&2
+  echo "imports is downloaded with it, however little of that a browser" >&2
+  echo "runs. SQL belongs in dist/beacon-rows.js, which no browser reaches." >&2
+  exit 1
+fi
+
 # The CLI, run out of the tarball.
 #
 # Running it proves more than that the file was packed. The extracted tarball

src/beacon-events.test.ts

@@ -13,13 +13,10 @@ import { deployStacks } from "#test/simulated-deployment.js";
 
 import { runAthenaQuery } from "./athena/athena-query.js";
 import {
-  aBeaconEvent,
-  beaconEventColumn,
   beaconParameters,
   beaconQueryString,
   beaconSchemaVersion,
   defaultBeaconPath,
-  outsideTheBeaconPath,
 } from "./beacon-events.js";
 import { CloudFrontLogDelivery } from "./cdk/log-delivery.js";
 import { LogBucket } from "./cdk/log-bucket.js";
@@ -75,35 +72,6 @@ describe("the beacon event envelope", () => {
     expect(sent.split("&")).toHaveLength(3);
   });
 
-  it("reads a row's event back off the column CloudFront wrote it to", () => {
-    // Given the SQL a rollup selects the event with.
-    // Then it reads the query string, which is where the payload is. No
-    // column of the table holds it, because a table column is a CloudFront
-    // field and CloudFront has no field for somebody else's payload.
-    expect(beaconEventColumn).toContain("cs_uri_query");
-    expect(beaconEventColumn).toContain(`'${beaconParameters.event}'`);
-  });
-
-  it("counts only requests carrying an envelope", () => {
-    // Given the conditions a rollup filters beacon rows with.
-    // Then a request to the beacon's path with no version parameter is left
-    // out. A crawler that found the URL in a page's source sends one of
-    // those, and counting it would report an event nobody caused.
-    expect(aBeaconEvent).toContain("cs_uri_query <> '-'");
-    expect(aBeaconEvent.join(" ")).toContain(`'${beaconParameters.version}'`);
-  });
-
-  it("names the path where the envelope's own conditions leave it out", () => {
-    // Given the condition status-codes takes the beacon's requests back out
-    // with.
-    // Then it names the path, where `aBeaconEvent` leaves the path to the
-    // request's own `paths`. The two run in opposite directions. It reads
-    // the column as delivered, since the path carries nothing a browser or
-    // CloudFront escapes.
-    expect(outsideTheBeaconPath).toContain(`'${defaultBeaconPath}'`);
-    expect(outsideTheBeaconPath).not.toContain("url_decode");
-  });
-
   it("sends to a path a site is unlikely to serve already", () => {
     // Given the default path.
     // Then it is one path, absolute, and marked as not a page. Pointing the

src/beacon-events.ts

@@ -1,5 +1,4 @@
-// What a beacon event is, and how it survives the round trip through a
-// CloudFront access log.
+// What a beacon event is, and the query string it travels in.
 //
 // The beacon sends a GET to a path on the site's own domain and puts its
 // payload in the query string. CloudFront records `cs-uri-query` whatever the
@@ -31,12 +30,11 @@
 // is for the beacon's own issue, and `version` is what lets that arrive
 // without reinterpreting rows already written.
 //
-// Both halves of the package read this. The beacon builds a query string from
-// it in a browser, so nothing here may reach a Node built-in or `aws-cdk-lib`,
-// and a rollup reads the same parameters back as SQL.
-
-import { decodedParameter } from "./log-encoding.js";
-import { quoted } from "./sql-text.js";
+// This module is the browser's half of that definition, and it imports
+// nothing. Every page of a measured site downloads it, so the SQL reading
+// these same parameters back off a row lives in `beacon-rows.ts` next door.
+// KensioSoftware/rainlytics#110 has what keeping the two together cost a
+// bundle.
 
 /**
  * The path a beacon reports to, where a site chooses none.
@@ -106,7 +104,7 @@ export interface BeaconEvent {
  *
  * The browser's own encoding, which is the single pass a request carries.
  * CloudFront adds its own on the way into the record, and
- * {@link beaconEventColumn} reads both back off.
+ * `beaconEventColumn` in `beacon-rows.ts` reads both back off.
  *
  * No leading `?`. The caller joins it to the path it is sending to.
  *
@@ -125,53 +123,3 @@ export function beaconQueryString(event: BeaconEvent): string {
     )
     .join("&");
 }
-
-/** The envelope version a row was written under, as SQL. */
-export const beaconVersionColumn = decodedParameter(beaconParameters.version);
-
-/** What happened, as SQL. */
-export const beaconEventColumn = decodedParameter(beaconParameters.event);
-
-/** The page it happened on, as SQL. */
-export const beaconPageColumn = decodedParameter(beaconParameters.page);
-
-/**
- * The rows a beacon event is, as conditions for `rowsFor`.
- *
- * The path is not among them. A rollup narrows to the beacon's path through
- * the request's own `paths`, the way any other question narrows to a section
- * of a site, and a site that moved its beacon then says so in one place.
- *
- * These leave out anything else reaching the same path. A crawler following
- * a beacon URL out of a page's source carries no version parameter, and the
- * bot filter `rowsFor` applies has already taken most of them.
- *
- * ```typescript
- * rowsFor({ ...request, paths: [defaultBeaconPath] }, aBeaconEvent);
- * ```
- */
-export const aBeaconEvent: readonly string[] = [
-  "cs_method = 'GET'",
-  "cs_uri_query <> '-'",
-  `${beaconVersionColumn} <> ''`,
-];
-
-/**
- * The rows outside the beacon's path, as a condition for `rowsFor`.
- *
- * The other direction from {@link aBeaconEvent}, and it names the path that
- * one leaves out. A question about beacon events narrows to the beacon
- * through the request's own `paths`. A question about what the site answered
- * has to take the beacon's requests back out, and `status-codes` is the one
- * that does.
- *
- * Matched against the column as CloudFront delivered it, where `--path`
- * decodes twice first. The path here is a constant this package chose and it
- * carries nothing a browser or CloudFront escapes, so a record holds it as it
- * was sent. An address somebody typed can hold anything.
- *
- * A prefix, the way every path match in Rainlytics is one.
- */
-export const outsideTheBeaconPath = `strpos(cs_uri_stem, ${quoted(
-  defaultBeaconPath,
-)}) <> 1`;

src/beacon-rollup.ts

@@ -30,12 +30,12 @@
 // exported for a site running the beacon to add to its own summaries, and a
 // deployment with no beacon computes nothing for it.
 
+import { defaultBeaconPath } from "./beacon-events.js";
 import {
   aBeaconEvent,
   beaconEventColumn,
   beaconPageColumn,
-  defaultBeaconPath,
-} from "./beacon-events.js";
+} from "./beacon-rows.js";
 import { qualifiedTableName } from "./dataset.js";
 import type { Rollup, RollupTotals } from "./rollups.js";
 import { rowsFor } from "./rollups.js";

src/beacon-rows.test.ts

@@ -0,0 +1,39 @@
+import { describe, expect, it } from "vitest";
+
+import { beaconParameters, defaultBeaconPath } from "./beacon-events.js";
+import {
+  aBeaconEvent,
+  beaconEventColumn,
+  outsideTheBeaconPath,
+} from "./beacon-rows.js";
+
+describe("reading a beacon event off a row", () => {
+  it("reads a row's event back off the column CloudFront wrote it to", () => {
+    // Given the SQL a rollup selects the event with.
+    // Then it reads the query string, which is where the payload is. No
+    // column of the table holds it, because a table column is a CloudFront
+    // field and CloudFront has no field for somebody else's payload.
+    expect(beaconEventColumn).toContain("cs_uri_query");
+    expect(beaconEventColumn).toContain(`'${beaconParameters.event}'`);
+  });
+
+  it("counts only requests carrying an envelope", () => {
+    // Given the conditions a rollup filters beacon rows with.
+    // Then a request to the beacon's path with no version parameter is left
+    // out. A crawler that found the URL in a page's source sends one of
+    // those, and counting it would report an event nobody caused.
+    expect(aBeaconEvent).toContain("cs_uri_query <> '-'");
+    expect(aBeaconEvent.join(" ")).toContain(`'${beaconParameters.version}'`);
+  });
+
+  it("names the path where the envelope's own conditions leave it out", () => {
+    // Given the condition status-codes takes the beacon's requests back out
+    // with.
+    // Then it names the path, where `aBeaconEvent` leaves the path to the
+    // request's own `paths`. The two run in opposite directions. It reads
+    // the column as delivered, since the path carries nothing a browser or
+    // CloudFront escapes.
+    expect(outsideTheBeaconPath).toContain(`'${defaultBeaconPath}'`);
+    expect(outsideTheBeaconPath).not.toContain("url_decode");
+  });
+});

src/beacon-rows.ts

@@ -0,0 +1,67 @@
+// Reading a beacon event back off the rows CloudFront wrote.
+//
+// Apart from `beacon-events.ts` because the two halves have different
+// readers. That module builds a query string inside a browser and this one
+// builds SQL for Athena, and a site folding the beacon into its own bundle
+// was carrying both. KensioSoftware/rainlytics#110 measured what that cost
+// and split them.
+//
+// The payload stays in `cs_uri_query` and is read at query time, the way
+// `searches` already reads a search term out of the same column. #100 settled
+// that, and `beacon-events.ts` carries the rest of the reasoning along with
+// the parameter names these expressions read.
+
+import { decodedParameter } from "./log-encoding.js";
+import { quoted } from "./sql-text.js";
+
+import { beaconParameters, defaultBeaconPath } from "./beacon-events.js";
+
+/** The envelope version a row was written under, as SQL. */
+export const beaconVersionColumn = decodedParameter(beaconParameters.version);
+
+/** What happened, as SQL. */
+export const beaconEventColumn = decodedParameter(beaconParameters.event);
+
+/** The page it happened on, as SQL. */
+export const beaconPageColumn = decodedParameter(beaconParameters.page);
+
+/**
+ * The rows a beacon event is, as conditions for `rowsFor`.
+ *
+ * The path is not among them. A rollup narrows to the beacon's path through
+ * the request's own `paths`, the way any other question narrows to a section
+ * of a site, and a site that moved its beacon then says so in one place.
+ *
+ * These leave out anything else reaching the same path. A crawler following
+ * a beacon URL out of a page's source carries no version parameter, and the
+ * bot filter `rowsFor` applies has already taken most of them.
+ *
+ * ```typescript
+ * rowsFor({ ...request, paths: [defaultBeaconPath] }, aBeaconEvent);
+ * ```
+ */
+export const aBeaconEvent: readonly string[] = [
+  "cs_method = 'GET'",
+  "cs_uri_query <> '-'",
+  `${beaconVersionColumn} <> ''`,
+];
+
+/**
+ * The rows outside the beacon's path, as a condition for `rowsFor`.
+ *
+ * The other direction from {@link aBeaconEvent}, and it names the path that
+ * one leaves out. A question about beacon events narrows to the beacon
+ * through the request's own `paths`. A question about what the site answered
+ * has to take the beacon's requests back out, and `status-codes` is the one
+ * that does.
+ *
+ * Matched against the column as CloudFront delivered it, where `--path`
+ * decodes twice first. The path here is a constant this package chose and it
+ * carries nothing a browser or CloudFront escapes, so a record holds it as it
+ * was sent. An address somebody typed can hold anything.
+ *
+ * A prefix, the way every path match in Rainlytics is one.
+ */
+export const outsideTheBeaconPath = `strpos(cs_uri_stem, ${quoted(
+  defaultBeaconPath,
+)}) <> 1`;

src/cdk/log-table.test.ts

@@ -10,13 +10,12 @@ import { describe, expect, it } from "vitest";
 import { deployStacks, simStartedAt } from "#test/simulated-deployment.js";
 
 import { defaultLogDataset, qualifiedTableName } from "../dataset.js";
+import { beaconQueryString, defaultBeaconPath } from "../beacon-events.js";
 import {
   beaconEventColumn,
   beaconPageColumn,
-  beaconQueryString,
   beaconVersionColumn,
-  defaultBeaconPath,
-} from "../beacon-events.js";
+} from "../beacon-rows.js";
 import {
   deliveredLogColumnNames,
   logFieldNamesWithoutAddress,

src/index.ts

@@ -74,17 +74,19 @@ export {
   windowRange,
 } from "./summary-runs.js";
 export {
-  aBeaconEvent,
   type BeaconEvent,
-  beaconEventColumn,
-  beaconPageColumn,
   beaconParameters,
   beaconQueryString,
   beaconSchemaVersion,
-  beaconVersionColumn,
   defaultBeaconPath,
-  outsideTheBeaconPath,
 } from "./beacon-events.js";
+export {
+  aBeaconEvent,
+  beaconEventColumn,
+  beaconPageColumn,
+  beaconVersionColumn,
+  outsideTheBeaconPath,
+} from "./beacon-rows.js";
 export { decodedColumn, decodedParameter } from "./log-encoding.js";
 export {
   lastRange,

src/rollup-questions.ts

@@ -6,7 +6,8 @@
 // rollup that filtered differently would answer a different question from
 // its neighbours without saying so.
 
-import { defaultBeaconPath, outsideTheBeaconPath } from "./beacon-events.js";
+import { defaultBeaconPath } from "./beacon-events.js";
+import { outsideTheBeaconPath } from "./beacon-rows.js";
 import { qualifiedTableName } from "./dataset.js";
 import { decodedColumn, decodedParameter } from "./log-encoding.js";
 import {

src/rollups.test.ts

@@ -3,7 +3,8 @@ import { describe, expect, it } from "vitest";
 
 import { defaultLogDataset, qualifiedTableName } from "./dataset.js";
 import { decodedParameter } from "./log-encoding.js";
-import { defaultBeaconPath, outsideTheBeaconPath } from "./beacon-events.js";
+import { defaultBeaconPath } from "./beacon-events.js";
+import { outsideTheBeaconPath } from "./beacon-rows.js";
 import { rollups } from "./rollup-questions.js";
 import type { Rollup, RollupRequest } from "./rollups.js";
 import {