From 89422d18ad7099f120bf4fc0cd49461f46108aa0 Mon Sep 17 00:00:00 2001 From: Marc Froehlich Date: Mon, 14 Sep 2026 22:40:53 +0200 Subject: [PATCH] Fix private analytics aggregation --- website/ops/analytics/private-metrics.js | 139 +++++++++++++++++------ website/test/analytics-generator.test.js | 44 ++++++- website/test/private-metrics.test.js | 29 ++++- 3 files changed, 171 insertions(+), 41 deletions(-) diff --git a/website/ops/analytics/private-metrics.js b/website/ops/analytics/private-metrics.js index 35ade4e5..3c8cab5f 100644 --- a/website/ops/analytics/private-metrics.js +++ b/website/ops/analytics/private-metrics.js @@ -76,8 +76,8 @@ function parseAnalyticsLine(line, timeZone = "Europe/Berlin") { const status = Number(fields[6]); const bytes = fields[7] === "-" ? 0 : Number(fields[7]); const local = localParts(fields[2], timeZone); - const requestPath = normalizePath(fields[4]); - if (!local || !requestPath || !/^\d{3}$/.test(fields[6]) + const requestPath = fields[4] === "" ? null : normalizePath(fields[4]); + if (!local || (fields[4] !== "" && !requestPath) || !/^\d{3}$/.test(fields[6]) || !Number.isSafeInteger(bytes) || bytes < 0) return null; let userAgent = fields[8]; if (userAgent.startsWith("\"") && userAgent.endsWith("\"")) { @@ -157,7 +157,7 @@ function isKnownBot(userAgent) { } function isEligibleWebsiteRequest(record) { - if (record.method !== "GET" || isKnownBot(record.userAgent)) return false; + if (record.method !== "GET" || !record.path || isKnownBot(record.userAgent)) return false; if (["/visitor-count.json", UPDATE_INFO_PATH, "/sitemap.xml", "/robots.txt", "/favicon.ico", "/assets/favicon.svg", "/health", "/healthz", "/ping", "/status"].includes(record.path)) { return false; @@ -270,17 +270,25 @@ function sumValues(values) { } function aggregateGoAccessReport(report, kind) { - const total = report && report.general && report.general.total_requests; - if (!Number.isSafeInteger(total) || total < 0) throw new Error("GoAccess report contains an invalid total request count"); + const validRequests = report && report.general && report.general.valid_requests; + if (!Number.isSafeInteger(validRequests) || validRequests < 0) { + throw new Error("GoAccess report contains an invalid valid-request count"); + } + const paths = countPanel(report, "requests"); + const total = sumValues(paths); + if (validRequests !== total) { + throw new Error("GoAccess valid-request count differs from the request-panel definition"); + } const countries = countCountries(report); const countryTotal = sumValues(countries); if (countryTotal > total) throw new Error("GoAccess country total exceeds the request total"); if (countryTotal < total) addCount(countries, UNKNOWN_COUNTRY, total - countryTotal); - if (kind === "updateInfo") return { requests: total, countries }; - const paths = countPanel(report, "requests"); - if (sumValues(paths) !== total) { - throw new Error("website page total differs from the GoAccess request-panel definition"); + if (kind === "updateInfo") { + if (Object.keys(paths).some(requestPath => requestPath !== UPDATE_INFO_PATH)) { + throw new Error("update-information report contains an unexpected request path"); + } + return { requests: total, countries }; } return { pageViews: total, countries, paths }; } @@ -299,6 +307,76 @@ function renderMetricsConfig(template, dbPath, geoIpCountryDatabase, includeCraw return rendered; } +function runMetricGoAccess({ records, jobId, metricKind, site, context, includeCrawlers }) { + const jobDirectory = path.join(context.runDirectory, jobId); + const dbPath = path.join(jobDirectory, "db"); + const inputPath = path.join(jobDirectory, "input.log"); + const outputJson = path.join(jobDirectory, "report.json"); + const runConfig = path.join(jobDirectory, "goaccess.conf"); + fs.mkdirSync(dbPath, { recursive: true }); + fs.writeFileSync(inputPath, `${records.map(record => record.line).join("\n")}\n`, { mode: 0o600 }); + fs.writeFileSync(runConfig, renderMetricsConfig( + context.configTemplate, + dbPath, + context.registry.geoIpCountryDatabase, + includeCrawlers + ), { mode: 0o600 }); + context.runGoAccess({ + binary: context.goaccessBinary, + args: [inputPath, "--no-global-config", "--config-file", runConfig, "--output", outputJson], + id: jobId, + metricKind, + date: records[0].date, + outputJson, + dbPath + }); + try { + return JSON.parse(fs.readFileSync(outputJson, "utf8")); + } catch (error) { + throw new Error(`private metric GoAccess JSON is invalid for ${site.id}/${records[0].date}: ${error.message}`); + } +} + +function filterWebsiteRecordsWithGoAccess(records, date, site, context) { + const representatives = []; + const pathByUserAgent = new Map(); + for (const record of records) { + if (pathByUserAgent.has(record.userAgent)) continue; + const classifierPath = `/__client/${pathByUserAgent.size}`; + pathByUserAgent.set(record.userAgent, classifierPath); + const representative = { + ...record, + ip: "192.0.2.1", + method: "GET", + path: classifierPath, + protocol: "HTTP/1.1", + status: 200, + bytes: 0 + }; + representative.line = toAnalyticsLine(representative); + representatives.push(representative); + } + const report = runMetricGoAccess({ + records: representatives, + jobId: `metrics-${site.id}-website-classifier-${date}`, + metricKind: "websiteClassifier", + site, + context, + includeCrawlers: false + }); + const acceptedPaths = countPanel(report, "requests"); + const knownPaths = new Set(pathByUserAgent.values()); + for (const [classifierPath, count] of Object.entries(acceptedPaths)) { + if (!knownPaths.has(classifierPath) || count !== 1) { + throw new Error(`website client classification is invalid for ${site.id}/${date}`); + } + } + const acceptedUserAgents = new Set([...pathByUserAgent] + .filter(([_userAgent, classifierPath]) => Object.hasOwn(acceptedPaths, classifierPath)) + .map(([userAgent]) => userAgent)); + return records.filter(record => acceptedUserAgents.has(record.userAgent)); +} + function aggregateDays({ records, dates, kind, site, context }) { const byDate = new Map(dates.map(date => [date, []])); for (const record of records) { @@ -306,7 +384,16 @@ function aggregateDays({ records, dates, kind, site, context }) { } const daily = {}; for (const date of dates) { - const dayRecords = byDate.get(date); + let dayRecords = byDate.get(date); + if (dayRecords.length === 0) { + daily[date] = kind === "website" + ? { pageViews: 0, countries: {}, paths: {} } + : { requests: 0, countries: {}, hours: {}, clients: {} }; + continue; + } + if (kind === "website") { + dayRecords = filterWebsiteRecordsWithGoAccess(dayRecords, date, site, context); + } if (dayRecords.length === 0) { daily[date] = kind === "website" ? { pageViews: 0, countries: {}, paths: {} } @@ -314,34 +401,14 @@ function aggregateDays({ records, dates, kind, site, context }) { continue; } const jobId = `metrics-${site.id}-${kind}-${date}`; - const jobDirectory = path.join(context.runDirectory, jobId); - const dbPath = path.join(jobDirectory, "db"); - const inputPath = path.join(jobDirectory, "input.log"); - const outputJson = path.join(jobDirectory, "report.json"); - const runConfig = path.join(jobDirectory, "goaccess.conf"); - fs.mkdirSync(dbPath, { recursive: true }); - fs.writeFileSync(inputPath, `${dayRecords.map(record => record.line).join("\n")}\n`, { mode: 0o600 }); - fs.writeFileSync(runConfig, renderMetricsConfig( - context.configTemplate, - dbPath, - context.registry.geoIpCountryDatabase, - kind === "updateInfo" - ), { mode: 0o600 }); - context.runGoAccess({ - binary: context.goaccessBinary, - args: [inputPath, "--no-global-config", "--config-file", runConfig, "--output", outputJson], - id: jobId, + const report = runMetricGoAccess({ + records: dayRecords, + jobId, metricKind: kind, - date, - outputJson, - dbPath + site, + context, + includeCrawlers: true }); - let report; - try { - report = JSON.parse(fs.readFileSync(outputJson, "utf8")); - } catch (error) { - throw new Error(`private metric GoAccess JSON is invalid for ${site.id}/${date}: ${error.message}`); - } daily[date] = aggregateGoAccessReport(report, kind); if (kind === "updateInfo") { const hours = {}; diff --git a/website/test/analytics-generator.test.js b/website/test/analytics-generator.test.js index 9552c8ca..cfa366fd 100644 --- a/website/test/analytics-generator.test.js +++ b/website/test/analytics-generator.test.js @@ -150,14 +150,18 @@ function fakeGoAccess(reports, calls, failureId) { throw new Error("simulated GoAccess failure"); } if (invocation.metricKind) { - const lines = fs.readFileSync(invocation.args[0], "utf8").trim().split(/\r?\n/); + const sourceLines = fs.readFileSync(invocation.args[0], "utf8").trim().split(/\r?\n/); + invocation.inputLines = sourceLines; + const lines = invocation.metricKind === "websiteClassifier" + ? sourceLines.filter(line => !line.includes("UnknownScanner/1.0")) + : sourceLines; const paths = {}; for (const line of lines) { const requestPath = line.split("\t")[4]; paths[requestPath] = (paths[requestPath] || 0) + 1; } const report = { - general: { total_requests: lines.length }, + general: { total_requests: lines.length, valid_requests: lines.length }, requests: { data: Object.entries(paths).map(([requestPath, count]) => ({ data: requestPath, @@ -181,6 +185,42 @@ function fakeGoAccess(reports, calls, failureId) { }; } +test("uses GoAccess client classification before aggregating website paths and countries", () => { + const testFixture = fixture([{ id: "alpha", publicCounter: true }]); + const calls = []; + const reports = { + alpha: goAccessReport({ "2026-09-11": 2 }), + combined: goAccessReport({ "2026-09-11": 2 }, true) + }; + fs.appendFileSync(testFixture.registry.sites[0].analyticsLog, analyticsLine({ + requestPath: "/scanner/", + userAgent: "UnknownScanner/1.0" + })); + try { + generateReports({ + registryPath: testFixture.registryPath, + configTemplatePath: testFixture.configTemplatePath, + goaccessBinary: "fake-goaccess" + }, { + checkGoAccess: () => GOACCESS_WITHOUT_ZLIB, + runGoAccess: fakeGoAccess(reports, calls), + now: () => new Date("2026-09-11T12:00:00Z"), + skipLock: true + }); + + const state = JSON.parse(fs.readFileSync(testFixture.registry.privateMetrics.statePath, "utf8")); + assert.equal(state.sites.alpha.website.daily["2026-09-11"].pageViews, 2); + assert.equal(state.sites.alpha.website.daily["2026-09-11"].paths["/scanner/"], undefined); + const classifierCall = calls.find(call => call.metricKind === "websiteClassifier"); + const aggregationCall = calls.find(call => call.metricKind === "website"); + assert.equal(classifierCall.inputLines.some(line => line.includes("UnknownScanner/1.0")), true); + assert.equal(classifierCall.inputLines.every(line => line.split("\t")[4].startsWith("/__client/")), true); + assert.equal(aggregationCall.inputLines.some(line => line.includes("UnknownScanner/1.0")), false); + } finally { + testFixture.cleanup(); + } +}); + function run(testFixture, reports, calls = [], failureId) { return generateReports({ registryPath: testFixture.registryPath, diff --git a/website/test/private-metrics.test.js b/website/test/private-metrics.test.js index 3103b420..04f7b930 100644 --- a/website/test/private-metrics.test.js +++ b/website/test/private-metrics.test.js @@ -57,6 +57,12 @@ test("parses analytics and regular Nginx combined records without query strings" assert.equal(analytics.hour, "23"); assert.equal(analytics.path, "/privacy/"); + const emptyPath = parseAnalyticsLine( + 'kst4contest.hamradioonline.de\t192.0.2.1\t2026-09-14T23:30:00+02:00\tGET\t\tHTTP/1.1\t400\t166\t"-"' + ); + assert.equal(emptyPath.path, null); + assert.equal(isEligibleWebsiteRequest(emptyPath), false); + const combined = parseCombinedLine( '192.0.2.2 - - [14/Sep/2026:23:31:00 +0200] "GET /news/?x=1 HTTP/1.1" 200 43 "-" "Mozilla/5.0 Firefox/130"', "kst4contest.hamradioonline.de" @@ -84,7 +90,7 @@ test("uses Europe/Berlin across both daylight-saving transitions", () => { test("keeps absolute countries including Switzerland, United Kingdom and unknown", () => { const website = aggregateGoAccessReport({ - general: { total_requests: 5 }, + general: { total_requests: 2245, valid_requests: 5 }, geolocation: { data: [ { data: "Europe", hits: { count: 3 }, items: [ { data: "Germany", hits: { count: 1 } }, @@ -107,10 +113,27 @@ test("keeps absolute countries including Switzerland, United Kingdom and unknown }); assert.deepEqual(website.paths, { "/": 2, "/privacy/": 2, "/news/": 1 }); assert.throws(() => aggregateGoAccessReport({ - general: { total_requests: 2 }, + general: { total_requests: 10, valid_requests: 2 }, geolocation: { data: [{ data: "Germany", hits: { count: 2 } }] }, requests: { data: [{ data: "/", hits: { count: 1 } }] } - }, "website"), /differs from the GoAccess request-panel definition/); + }, "website"), /valid-request count differs from the request-panel definition/); + + const updates = aggregateGoAccessReport({ + general: { total_requests: 9, valid_requests: 2 }, + geolocation: { data: [{ data: "Germany", hits: { count: 2 } }] }, + requests: { data: [{ data: "/kst4ContestVersionInfo.xml", hits: { count: 2 } }] } + }, "updateInfo"); + assert.equal(updates.requests, 2); + assert.throws(() => aggregateGoAccessReport({ + general: { total_requests: 2, valid_requests: 1 }, + geolocation: { data: [{ data: "Germany", hits: { count: 2 } }] }, + requests: { data: [{ data: "/", hits: { count: 1 } }] } + }, "website"), /country total exceeds/); + assert.throws(() => aggregateGoAccessReport({ + general: { total_requests: 1, valid_requests: 1 }, + geolocation: { data: [{ data: "Germany", hits: { count: 1 } }] }, + requests: { data: [{ data: "/unexpected", hits: { count: 1 } }] } + }, "updateInfo"), /unexpected request path/); }); test("deduplicates overlapping import files and reads gzip without GoAccess Zlib", () => {