ClickHouse usage analytics: events/gauges tables with daily MV

[lohanidamodar]

Summary

Complete rewrite of the usage analytics library with a two-table architecture optimized for both real-time analytics and billing.

Architecture

• Events table (MergeTree) — raw request events with dedicated columns for path, method, status, resource, resourceId, country (LowCardinality), userAgent
• Gauges table (MergeTree) — simple resource snapshots (metric, value, time, tags)
• Daily MV (SummingMergeTree) — pre-aggregates events by metric + tenant + day for fast billing
• No periods — query-time aggregation via toStartOfHour/toStartOfDay instead of write-time fan-out
• Single write path — collect(metric, value, type, tags) routes to correct table; event columns auto-extracted from tags

Key Changes

• Two separate tables instead of one — events have 7 extra columns gauges don't need
• Plain MergeTree for both tables — raw appends, query-time aggregation
• Daily MV with minimal schema (metric, value, time, tenant)
• LowCardinality(String) for country column
• Bloom filter indexes on all filterable columns
• String tenant — setTenant(?string)
• Utopia Query for all read operations — parameterized queries, no SQL injection risk

API

Write

• collect(metric, value, type, tags) — buffer with auto-flush
• addBatch(metrics, type) — direct batch insert
• flush() — write buffered metrics

Read

• find(queries, type) / count(queries, type) / sum(queries, attr, type)
• getTotal(metric, queries, type) — SUM for events, argMax for gauges
• getTotalBatch(metrics, queries, type) — batch totals
• getTimeSeries(metrics, interval, start, end, queries, zeroFill, type)

Billing (Daily MV)

• findDaily(queries) / sumDaily(queries) / sumDailyBatch(metrics, queries)

Test Plan

• Unit tests for Metric schema, getters, validation
• Integration tests for ClickHouse and Database adapters
• PHPStan level max passing
• Linter passing
• Security audit — no SQL injection vulnerabilities
• CI green (CodeQL, Tests, Linter)

[loks0n]

This seems specific to Database adapter, should we move it to Databases.php?

[loks0n]

We're going to need a separate HttpLog.php type anyways, do we need these properties?

[greptile-apps]

Cross-type attribute fallback passes invalid columns to gauge queries

validateAttributeName() falls back to checking the opposite type's attributes. When find($queries, null) (or count/sum with $type = null) is called with event-specific filters like Query::equal('path', '/v1/'), parseQueries forwards the same $queries to both tables. For the gauge call, validateAttributeName('path', 'gauge') passes because it finds path in the event schema, generating WHERE \path` = {param:String}against the gauges table — which has no such column — causing a ClickHouseUNKNOWN_IDENTIFIER` server error at runtime.

The cross-type fallback should be removed so that an attribute is only valid if it exists in the target table's own schema:

// Remove lines 1138-1144 (the "Also check the other type's attributes" block)
throw new Exception("Invalid attribute name: {$attributeName}");

Any cross-table query with type-specific filters should require the caller to pass an explicit $type.

[greptile-apps]

Cross-type attribute fallback sends invalid columns to gauge queries

validateAttributeName falls back to checking the opposite type's schema (lines 1139–1144). When find/count/sum is called with $type = null, parseQueries passes the same queries to both tables. A caller passing Query::equal('path', '/v1/') passes validation for the gauge table because path is found in the event schema, generating WHERE \path` = {param:String}against the gauges table — which has no such column — and causing a ClickHouseUNKNOWN_IDENTIFIER` server error at runtime.

Remove the fallback block so each attribute is only valid for its own table's schema:

Suggested change

	// Also check the other type's attributes for cross-table queries
	$otherType = $type === 'event' ? 'gauge' : 'event';
	foreach ($this->getAttributes($otherType) as $attribute) {
	if ($attribute['$id'] === $attributeName) {
	return true;
	}
	}
	throw new Exception("Invalid attribute name: {$attributeName}");
	}

[greptile-apps]

count(null, $max) returns up to 2 × $max, violating the documented upper-bound contract

When $type is null, count() independently applies $max as a SQL LIMIT to each of the events and gauges tables, then adds the two results. With count([], null, 100) a caller receives up to 200 — not at most 100. The Adapter docblock is explicit: $max is an "upper bound for the count (inclusive)". Fix: query both tables without per-table limit and cap the sum at the end.

[greptile-apps]

parseAggregatedResults truncates aggregate values to int

(int) $value silently truncates ClickHouse-computed aggregates. getTimeSeriesFromTable correctly uses (float) at line 2095; parseAggregatedResults and parseResults (line 2924) are inconsistent and will silently lose data if the schema is ever widened to Float64.

[greptile-apps]

MergeTree ORDER BY (id) eliminates time-range query efficiency

Both the events and gauges tables are created with ORDER BY (id) (or (tenant, id)), where id is a randomly generated UUID. ClickHouse stores rows physically in this sort order and uses it as the primary key for granule skipping. Because UUIDs are random, all time-range queries like WHERE time BETWEEN X AND Y and WHERE metric = 'foo' AND time BETWEEN ... will scan every granule — effectively a full table scan — at any scale.

The daily table correctly uses ORDER BY (metric, time) (line 1060), enabling efficient per-metric range scans. The raw events and gauges tables need the same treatment. A typical ORDER BY for this schema would be (metric, toStartOfDay(time), id) or (tenant, metric, toStartOfDay(time)) for shared tables, so ClickHouse can skip entire months/days worth of granules for a given metric.

[greptile-apps]

getTimeSeries with $type=null produces incoherent merged time series

When $type is null, results from both tables are fetched — events with SUM(value) per bucket, gauges with argMax(value, time) per bucket — and then array_merge'd into the same data array. If a metric name exists in both tables, zeroFillTimeSeries then sums the two incompatible values in the same bucket ($existing[$key] = ($existing[$key] ?? 0) + $point['value']), producing a meaningless total that mixes a cumulative sum with a point-in-time snapshot.

getTotal() guards against this (throws when $eventTotal > 0 && $gaugeTotal > 0), but getTimeSeries has no equivalent guard. Consider applying the same check before merging, or throwing when both result sets are non-empty for the same metric.

[greptile-apps]

find($queries, null) with a limit Query returns up to 2× the requested limit

When $type is null, findFromTable is called for both tables with the same parsed queries. The limit is extracted once by parseQueries and applied independently to each SQL query (line 1535: LIMIT {limit:UInt64}). A call to find([Query::limit(10)], null) can therefore return up to 20 results — 10 from events, 10 from gauges. This violates the documented semantics of Query::limit as an upper bound and mirrors the same off-by-factor-of-two bug already flagged for count() with $type=null.

The same issue applies to offset pagination: find([Query::offset(10), Query::limit(5)], null) skips the first 10 rows in each table independently, not the first 10 rows of the combined result set.

[greptile-apps]

findDaily() queries SummingMergeTree without SUM + GROUP BY, returning stale unmerged rows

findDaily() issues a plain SELECT metric, value, time FROM <daily_table> WHERE .... SummingMergeTree only sums rows with the same ORDER BY key (metric, time) during background merges. Between an INSERT and the next merge, multiple rows can exist for the same (metric, time) with separate values. Callers receive raw, unmerged rows instead of the correct daily aggregate — billing queries would undercount or overcount depending on timing.

The SummingMergeTree contract requires reading with SUM(value) GROUP BY (metric, time):

$sql = "SELECT metric, SUM(value) as value, time FROM {$fromTable}{$whereData['clause']} GROUP BY metric, time {$orderClause}{$limitClause}{$offsetClause} FORMAT JSON";

sumDaily() and sumDailyBatch() already use the correct SUM + GROUP BY pattern; findDaily() is the only outlier.

[greptile-apps]

User-supplied Query::orderDesc('time') in aggregated mode references a non-existent column

findAggregatedFromTable() aliases the time bucket as bucket in the SELECT and GROUP BY, but the user-supplied ORDER BY clause is built from parseQueries() using the original attribute names (e.g., `time` DESC). If a caller passes both UsageQuery::groupByInterval('1h') and Query::orderDesc('time'), the generated SQL becomes:

SELECT metric, SUM(value) as value, toStartOfHour(time) as bucket
FROM …
GROUP BY metric, bucket
ORDER BY `time` DESC   -- `time` is not a grouped/aggregated column here

ClickHouse raises an error because time is neither in the GROUP BY list nor an aggregate function result after grouping. The fix is to remap time ORDER BY entries to bucket in the aggregated query.

[greptile-apps]

Negative-value constraint absent in Database adapter

ClickHouse::validateMetricData() (line 1293) throws when $value < 0, and Usage::collect() enforces the same rule. However, Database::addBatch() has no corresponding guard. Calling $adapter->addBatch([['metric' => 'x', 'value' => -1]], Usage::TYPE_EVENT) on a Database adapter succeeds silently, while the identical call on a ClickHouse adapter throws. Code that passes the test suite with the Database adapter can therefore fail in production with the ClickHouse adapter. The abstract Adapter::addBatch() docblock does not document the sign constraint, so callers have no indication of the difference.

[greptile-apps]

getTimeSeries($queries, null) throws when event-specific filters are present

getTimeSeries with $type = null iterates over both TYPE_EVENT and TYPE_GAUGE. For each type it calls getTimeSeriesFromTable, which calls parseQueries($queries, $queryType). When $queryType is TYPE_GAUGE, validateAttributeName rejects any event-only column (path, method, status, etc.) and throws "Invalid attribute name: path". A caller that passes Query::equal('path', '/v1/') with $type = null will always get an exception on the gauge pass, even though the filter is perfectly valid for the event table. The same pattern was already flagged for find()/count()/sum() but getTimeSeries() has the identical code path and is not guarded.

[greptile-apps]

getTimeSeries($type=null) accumulates incoherent gauge+event totals

When $type is null, both tables are queried: events with SUM(value) per bucket and gauges with argMax(value, time) per bucket. The results are merged into $output[$metricName]['total'] += $metricData['total'], and then zeroFillTimeSeries sums both contributions into the same bucket via $existing[$key] += $point['value']. If a metric name exists in both tables, the resulting total is the sum of a cumulative counter and a point-in-time snapshot — a meaningless number. getTotal() throws in the analogous situation; getTimeSeries() silently corrupts the result.