diff --git a/.oxlintrc.json b/.oxlintrc.json index 8af40ea..0b7ab88 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -15,9 +15,7 @@ // rule list, and the beacon's function is the one file they cover. A repo // told to use no arrow functions anywhere would be unusable, so the rules // arrive as an override rather than at the top level. - "extends": [ - "./node_modules/@kensio/yulin/dist/config/oxlint/cffjs2.oxlintrc.json" - ], + "extends": ["./node_modules/@kensio/yulin/cffjs2.oxlintrc.json"], "plugins": ["eslint", "typescript", "unicorn", "oxc", "vitest", "jsdoc"], diff --git a/package.json b/package.json index 939d235..2eabb2f 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "@aws-sdk/client-ssm": "^3.1119.0", "@faker-js/faker": "^10.6.0", "@kensio/smartass": "^1.37.5", - "@kensio/yulin": "^1.20.14", + "@kensio/yulin": "^1.21.0", "@semantic-release/exec": "7.1.0", "@types/node": "^26.1.1", "@typescript/native": "npm:typescript@^7.0.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 84c8508..e1b0335 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -25,8 +25,8 @@ importers: specifier: ^1.37.5 version: 1.37.5 '@kensio/yulin': - specifier: ^1.20.14 - version: 1.20.14(node-sql-parser@5.4.0) + specifier: ^1.21.0 + version: 1.21.0(node-sql-parser@5.4.0) '@semantic-release/exec': specifier: 7.1.0 version: 7.1.0(semantic-release@25.0.9(@typescript/typescript6@6.0.2)(supports-color@7.2.0))(supports-color@7.2.0) @@ -231,8 +231,8 @@ packages: resolution: {integrity: sha512-Djp3HOC3s3U3axh/Phb31vJ1QCYls5DasmPAJZ466ArFE2aed5a1zQCHVLh3sCP7BjER/YRqBXqlRIwVpG+wkw==} engines: {node: '>=24.0.0'} - '@kensio/yulin@1.20.14': - resolution: {integrity: sha512-vao/M2vY9BsMbzZ7tVh1B1mfr/Na1LbDzKzqF7svw7G1y4FWqHpRUAWqNKhfZmMOXZPu88Tvo3e8jOBIspgjOA==} + '@kensio/yulin@1.21.0': + resolution: {integrity: sha512-pG+cXsnY+VEIvH48sYWiZLkU9AMyfSiSDgwmCoLMRS8ehJWaD+vPuFb2JxSz0aWbJOZ4bUb+oQCIAvfHbZMJaw==} engines: {node: '>=24.0.0'} hasBin: true peerDependencies: @@ -2621,7 +2621,7 @@ snapshots: '@kensio/smartass@1.37.5': {} - '@kensio/yulin@1.20.14(node-sql-parser@5.4.0)': + '@kensio/yulin@1.21.0(node-sql-parser@5.4.0)': dependencies: '@faker-js/faker': 10.6.0 '@kensio/part-factory': 1.10.1 diff --git a/src/beacon-rollup.ts b/src/beacon-rollup.ts index 9fa4acb..e33a8a5 100644 --- a/src/beacon-rollup.ts +++ b/src/beacon-rollup.ts @@ -87,18 +87,8 @@ const sender = "c_ip, cs_user_agent"; */ const loggedHour = "cast(timestamp_ms AS bigint) / 3600000"; -/** - * One visitor's identical events in one hour, counted no further than the cap. - * - * `least(count(*), 60)` says this in fewer characters and is what Athena would - * take. Yulin's query engine has no `least` and answers the whole query from a - * declaration rather than running it, which would leave this rule with no test - * that executes. Raised as KensioSoftware/yulin#1141. The `CASE` is the same - * arithmetic in a form both engines run. - */ -const cappedCount = - `CASE WHEN count(*) > ${String(beaconEventCap)}` + - ` THEN ${String(beaconEventCap)} ELSE count(*) END`; +/** One visitor's identical events in one hour, counted no further than the cap. */ +const cappedCount = `least(count(*), ${String(beaconEventCap)})`; /** Events by page and by name, added by both. */ const beaconEventTotals: RollupTotals = { added: ["events"] }; diff --git a/src/cdk/beacon-path.test.ts b/src/cdk/beacon-path.test.ts index 18aa85a..e1c3932 100644 --- a/src/cdk/beacon-path.test.ts +++ b/src/cdk/beacon-path.test.ts @@ -4,8 +4,7 @@ import { CachePolicy, Distribution } from "aws-cdk-lib/aws-cloudfront"; import { S3BucketOrigin } from "aws-cdk-lib/aws-cloudfront-origins"; import { PolicyStatement, ServicePrincipal } from "aws-cdk-lib/aws-iam"; import { Bucket } from "aws-cdk-lib/aws-s3"; -import { Match, Template } from "aws-cdk-lib/assertions"; -import { App, CfnOutput, RemovalPolicy, Stack } from "aws-cdk-lib/core"; +import { type App, CfnOutput, RemovalPolicy, Stack } from "aws-cdk-lib/core"; import { describe, expect, it } from "vitest"; import { deployStacks } from "#test/simulated-deployment.js"; @@ -58,6 +57,9 @@ describe("answering the beacon's collection path", () => { new CfnOutput(stack, "DistributionDomainName", { value: distribution.distributionDomainName, }); + new CfnOutput(stack, "DistributionId", { + value: distribution.distributionId, + }); new CfnOutput(stack, "SiteBucketName", { value: bucketName }); new BeaconPath(stack, "RainlyticsBeacon", { @@ -76,6 +78,7 @@ describe("answering the beacon's collection path", () => { return { simAws, bucketName, + distributionId: stack?.output("DistributionId") ?? "", get: async (pathAndQuery: string): Promise => http.fetch(`https://${host}${pathAndQuery}`, { redirect: "manual" }), }; @@ -190,125 +193,90 @@ describe("answering the beacon's collection path", () => { }); describe("what the distribution is given", () => { - /** - * These read the synthesised template rather than the deployed - * simulation. Yulin models no cache policy on a behaviour and its - * CloudFront has no `ListFunctions` or `GetFunction`, so a deployed - * distribution has nothing to ask about either. Raised as - * KensioSoftware/yulin#1130 and KensioSoftware/yulin#1131. - * - * What the deployed function does is covered above, by requests going - * into the simulation and coming back 204. - */ - const synthesised = (props: Partial = {}): Template => { - const stack = new Stack(new App(), "SiteStack", { - env: { account: "123456789012", region: "eu-west-2" }, - }); - new Bucket(stack, "SiteBucket", { bucketName: "site-bucket" }); - const origin = S3BucketOrigin.withOriginAccessControl( - Bucket.fromBucketName(stack, "SiteOrigin", "site-bucket"), - ); - const distribution = new Distribution(stack, "SiteDistribution", { - defaultBehavior: { origin }, - }); - new BeaconPath(stack, "RainlyticsBeacon", { - ...props, - distribution, - origin, - }); - - return Template.fromStack(stack); - }; - /** The managed `CachingOptimized` policy, under the fixed id AWS gives it. */ const cachingOptimized = "658327ea-f89d-4fab-a63d-7e88639e58f6"; - /** The distribution properties, matched on the beacon's own behaviour. */ - const beaconBehaviour = ( - properties: Record, - ): Record => { - const matched = Match.objectLike({ - PathPattern: defaultBeaconPath, - ...properties, - }); + /** The beacon's own behaviour, read off the deployed distribution. */ + const beaconBehaviour = async (props: Partial = {}) => { + const { simAws, distributionId } = await deployBeacon(props); + const read = await simAws + .cloudFront() + .getDistribution({ input: { Id: distributionId } }); - return { - DistributionConfig: Match.objectLike({ - CacheBehaviors: Match.arrayWith([matched]), - }), - }; + return read.Distribution?.DistributionConfig?.CacheBehaviors?.Items?.find( + (behaviour) => + behaviour.PathPattern === (props.path ?? defaultBeaconPath), + ); }; - it("leaves the query string out of the cache key", () => { + /** The one function the deployed account holds, live. */ + const publishedFunction = async (props: Partial = {}) => { + const { simAws } = await deployBeacon(props); + const listed = await simAws + .cloudFront() + .listFunctions({ input: { Stage: "LIVE" } }); + + return listed.FunctionList.Items[0]; + }; + + it("leaves the query string out of the cache key", async () => { // Given a beacon path taking the default cache policy. - // When the stack is synthesised. - const template = synthesised(); + // When the stack is deployed. + const behaviour = await beaconBehaviour(); // Then the behaviour carries the managed policy that keys on the path // alone. The payload travels in the query string, and a policy keying // on it would make every event a cache entry of its own. - template.hasResourceProperties( - "AWS::CloudFront::Distribution", - beaconBehaviour({ CachePolicyId: cachingOptimized }), - ); + expect(behaviour?.CachePolicyId).toBe(cachingOptimized); }); - it("takes a cache policy a site would rather use", () => { + it("takes a cache policy a site would rather use", async () => { // Given a site standardising on one managed policy across its // behaviours, this one keying on nothing and storing nothing. const cachePolicy = CachePolicy.CACHING_DISABLED; // When the beacon is deployed with it. - const template = synthesised({ cachePolicy }); + const behaviour = await beaconBehaviour({ cachePolicy }); // Then that is the policy on the behaviour. - template.hasResourceProperties( - "AWS::CloudFront::Distribution", - beaconBehaviour({ CachePolicyId: cachePolicy.cachePolicyId }), - ); + expect(behaviour?.CachePolicyId).toBe(cachePolicy.cachePolicyId); }); - it("runs the function before the cache is consulted", () => { + it("runs the function before the cache is consulted", async () => { // Given the same stack. - const template = synthesised(); + const behaviour = await beaconBehaviour(); // Then the function is associated at viewer-request. CloudFront // reaches that event before the cache lookup and before any origin // request, and it is what makes the 204 free of both. - const atViewerRequest = Match.objectLike({ - EventType: "viewer-request", - }); - template.hasResourceProperties( - "AWS::CloudFront::Distribution", - beaconBehaviour({ FunctionAssociations: [atViewerRequest] }), - ); + expect( + behaviour?.FunctionAssociations?.Items?.map( + (association) => association.EventType, + ), + ).toStrictEqual(["viewer-request"]); }); - it("deploys the function on the JS 2.0 runtime", () => { + it("deploys the function on the JS 2.0 runtime", async () => { // Given the same stack. - const template = synthesised(); + const summary = await publishedFunction(); // Then the function names the runtime its source is written against. // The lint rules on `beacon-204.cff.js` hold it to JS 2.0's // restrictions, and JS 1.0 has its own. - template.hasResourceProperties("AWS::CloudFront::Function", { - FunctionConfig: Match.objectLike({ Runtime: "cloudfront-js-2.0" }), - }); + expect(summary?.FunctionConfig.Runtime).toBe("cloudfront-js-2.0"); }); - it("takes a function name where the account needs a chosen one", () => { + it("takes a function name where the account needs a chosen one", async () => { // Given two sites in one account. CloudFront function names are unique // across an account, and CDK derives one from the construct's path in // the tree. const functionName = `beacon-${faker.string.alphanumeric(8)}`; // When the beacon is deployed under a name of its own. - const template = synthesised({ functionName }); + const summary = await publishedFunction({ functionName }); // Then that is the name it carries. - template.hasResourceProperties("AWS::CloudFront::Function", { - Name: functionName, - }); + expect(summary?.Name).toBe(functionName); }); }); }); diff --git a/src/cdk/log-delivery.test.ts b/src/cdk/log-delivery.test.ts index e5810d6..3616732 100644 --- a/src/cdk/log-delivery.test.ts +++ b/src/cdk/log-delivery.test.ts @@ -67,8 +67,8 @@ describe("delivering CloudFront access logs", () => { * The distribution is deployed rather than invented. A delivery source * names a distribution by ARN, and AWS refuses one naming a distribution * that does not exist, so a fabricated id makes the whole delivery a thing - * production would have rejected. See KensioSoftware/yulin#993, which is - * about the simulation catching up to that. + * production would have rejected. Simulated CloudWatch Logs refuses it as + * well, since KensioSoftware/yulin#993. * * Everything goes in one us-east-1 stack. A real consumer keeps the * distribution wherever their site is, and the cases about that split are diff --git a/src/cli/summary-answer.test.ts b/src/cli/summary-answer.test.ts index 580297f..519e012 100644 --- a/src/cli/summary-answer.test.ts +++ b/src/cli/summary-answer.test.ts @@ -19,7 +19,7 @@ import { RollupSummaries } from "../cdk/rollup-summaries.js"; import type { RollupSummariesProps } from "../cdk/summary-configuration.js"; import { partitionPrefix } from "../partitions.js"; import { cacheHitRatio, pageviews } from "../rollup-questions.js"; -import type { Rollup } from "../rollups.js"; +import { defaultVisitorSaltParameter } from "../visitor-identity.js"; import { rainlyticsCommands } from "./command.js"; import { runCli } from "./run.js"; @@ -35,18 +35,6 @@ import { runCli } from "./run.js"; describe("the named questions, answered from stored summaries", () => { let intercepted: SimSdk | undefined; - /** - * The pageviews question with its visitor count turned off. - * - * These cases are about reading a stored answer back, and a visitor count - * would be a second query nothing here can answer. Yulin's Athena engine - * has no `sha256`, `to_utf8` or `to_hex`, so the count comes back empty - * under a SUCCEEDED state and the run refuses it, leaving no summary to - * read. KensioSoftware/yulin#1082 is that gap. The name is the same, so - * every key and question below is the one a shipped deployment writes. - */ - const viewsOnly: Rollup = { ...pageviews, countsVisitors: false }; - /** The hour the traffic in these cases happened in. */ const anHour = new Date("2026-08-23T08:00:00.000Z"); @@ -94,7 +82,7 @@ describe("the named questions, answered from stored summaries", () => { new RollupSummaries(stack, "RainlyticsSummaries", { table, workgroup, - rollups: [viewsOnly, cacheHitRatio], + rollups: [pageviews, cacheHitRatio], granularities: ["hourly"], summariesBucketName, removalPolicy: RemovalPolicy.DESTROY, @@ -114,6 +102,22 @@ describe("the named questions, answered from stored summaries", () => { intercepted.intercept(AthenaClient); intercepted.intercept(S3Client); + // The salt secret, put where a site's operator puts it. Nothing in the + // stack creates it, because CloudFormation writes no SecureString. + // `docs/visitors/` has the command. The summaries below carry a visitor + // count, and the schedules need it before they fire. + await simAws + .region("us-east-1") + .account() + .ssm() + .putParameter({ + input: { + Name: defaultVisitorSaltParameter, + Type: "SecureString", + Value: faker.string.hexadecimal({ length: 64, prefix: "" }), + }, + }); + return { simAws, logBucketName, diff --git a/src/functions/rollup-summary.test.ts b/src/functions/rollup-summary.test.ts index 9a986eb..760fbc6 100644 --- a/src/functions/rollup-summary.test.ts +++ b/src/functions/rollup-summary.test.ts @@ -29,6 +29,7 @@ import { summarisedWindow, windowPlaceholder, } from "../rollups.js"; +import { visitorCountSql } from "../visitor-counts.js"; import { defaultVisitorSaltParameter } from "../visitor-identity.js"; import { visitorSaltPlaceholder } from "../visitor-identity.js"; import { handler } from "./rollup-summary.js"; @@ -178,20 +179,13 @@ describe("one run of the rollup summary job", () => { }); /** - * A visitor count the simulated engine can answer. + * The visitor count a schedule carries beside the pageviews question. * - * The shipped one is `count(DISTINCT to_hex(sha256(to_utf8(...))))`, and - * Yulin has neither the digest nor a distinct count over an expression. - * KensioSoftware/yulin#1082 is that gap. This counts distinct addresses - * over the same window and carries the salt where the shipped query carries - * it, so what these cases cover is the run around the count. + * The shipped query, narrowed the way `aRun` narrows the question above. * `visitor-counts.test.ts` covers who one identifier stands for. */ const aVisitorCount = (): string => - `SELECT count(DISTINCT c_ip) AS visitors\n` + - ` FROM "rainlytics"."cloudfront_logs"\n` + - ` WHERE ${windowPlaceholder}\n` + - ` AND ${visitorSaltPlaceholder} <> ''\n`; + visitorCountSql(rollupRequest({ range: summarisedWindow })); /** * A quarter past nine, on both clocks. diff --git a/src/visitor-counts.test.ts b/src/visitor-counts.test.ts index 54a15f1..cb5645a 100644 --- a/src/visitor-counts.test.ts +++ b/src/visitor-counts.test.ts @@ -33,7 +33,7 @@ import { import { saltedSql, visitorSaltPlaceholder, - visitorText, + visitorIdentifier, } from "./visitor-identity.js"; describe("the query that counts visitors", () => { @@ -72,14 +72,9 @@ describe("the query that counts visitors", () => { /* * What one record hashes to, run through Athena. * - * These count the distinct text a digest is taken over rather than the digest - * itself. Yulin's Athena engine has no `sha256`, `to_utf8` or `to_hex`, and a - * query reaching for one comes back empty under a SUCCEEDED state. - * KensioSoftware/yulin#1082 is that gap. - * - * `sha256` is injective for these purposes, so two texts are two identifiers - * and one text is one. What the cases below establish about the text - * therefore holds of the identifier over it. + * These select the identifier itself over the rows the shipped count reads. + * Simulated Athena computes the digest. The identifiers here are the ones a + * deployment counts. */ describe("who one identifier stands for", () => { let intercepted: SimSdk | undefined; @@ -175,21 +170,20 @@ describe("who one identifier stands for", () => { }; /** - * The distinct texts one window hashes, under one day's salt. + * The distinct identifiers one window holds, under one day's salt. * * The account is the one `deployAnalytics` deployed. Every case here builds * one before it seeds anything, and the SDK is pointed at it there. */ - const textsIn = async ( + const identifiersIn = async ( window: SummaryWindow, salt: string, ): Promise => { - // The rows the shipped count reads, with the text a digest is taken over - // put where the count would be. Yulin cannot evaluate the digest, and it - // runs everything under it. + // The rows the shipped count reads, with the identifier it counts put + // where the count would be. const request = rollupRequest({ range: summarisedWindow }); const lines = [ - `SELECT DISTINCT ${visitorText} AS visitor`, + `SELECT DISTINCT ${visitorIdentifier} AS visitor`, ` FROM ${qualifiedTableName()}`, visitorRows(request), ]; @@ -218,7 +212,9 @@ describe("who one identifier stands for", () => { await putView(deployed, anHour, "203.0.113.7"); // Then the hour holds one visitor. - await expect(textsIn(anHourly(anHour), "a-salt")).resolves.toHaveLength(1); + await expect( + identifiersIn(anHourly(anHour), "a-salt"), + ).resolves.toHaveLength(1); }); it("is two identifiers for two addresses on one day", async () => { @@ -229,9 +225,9 @@ describe("who one identifier stands for", () => { await putView(deployed, anHour, "198.51.100.24"); // Then the hour holds two visitors. - const texts = await textsIn(anHourly(anHour), "a-salt"); + const identifiers = await identifiersIn(anHourly(anHour), "a-salt"); - expect(new Set(texts).size).toBe(2); + expect(new Set(identifiers).size).toBe(2); }); it("is two identifiers for one address on two days", async () => { @@ -243,8 +239,8 @@ describe("who one identifier stands for", () => { // When each day is counted under its own salt. const [today, tomorrow] = await Promise.all([ - textsIn(anHourly(anHour), "todays-salt"), - textsIn(anHourly(nextDay), "tomorrows-salt"), + identifiersIn(anHourly(anHour), "todays-salt"), + identifiersIn(anHourly(nextDay), "tomorrows-salt"), ]); // Then the same person is a different visitor. This is why a month is not @@ -262,13 +258,12 @@ describe("who one identifier stands for", () => { await putView(deployed, anHour); // When the hour is counted. - const texts = await textsIn(anHourly(anHour), "a-salt"); + const identifiers = await identifiersIn(anHourly(anHour), "a-salt"); - // Then only the record with an address is counted. Left in, every record - // of the days before the delivery changed would gather into one - // identifier and report a visitor nobody was. - expect(texts).toStrictEqual([ - "a-salt|203.0.113.7|Mozilla/5.0%20(Macintosh)", - ]); + // Then only the record with an address is counted. The addressless one + // hashes to an identifier of its own, and leaving it in would show up + // here as a second visitor. Every record of the days before the delivery + // changed would gather into that one and report somebody nobody was. + expect(identifiers).toHaveLength(1); }); });