Skip to content

feat(tests): add tests for low and high cardinality request attribute… #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ArnabChatterjee20k wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat(tests): add tests for low and high cardinality request attribute… #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9827477
feat(tests): add tests for low and high cardinality request attribute…
ArnabChatterjee20k Sep 3, 2026
f9c2e4f
test(clickhouse): sync schema-test cardinality list with new columns
ArnabChatterjee20k Sep 3, 2026
139d361
feat(metric): add skip indexes for new filterable request/geo columns
ArnabChatterjee20k Sep 7, 2026
d6dcb62
fix(metric): prefix-index oversized accept/queryKeys columns for SQL …
ArnabChatterjee20k Sep 7, 2026
aa5858a
feat(metric): treat accept/acceptLanguage/queryKeys as low-cardinalit…
ArnabChatterjee20k Sep 10, 2026
688a5fe
chore: restore composer.lock (accidental local composer update in aa5…
ArnabChatterjee20k Sep 10, 2026
322a481
feat(metric): keep accept/acceptLanguage/queryKeys high-cardinality w…
ArnabChatterjee20k Sep 10, 2026
cc1fd86
feat(metric): add ipReputation placeholder column (bounded verdict enum)
ArnabChatterjee20k Sep 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/Usage/Adapter/ClickHouse.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1791,10 +1791,19 @@ private function getColumnType(string $id, string $type = 'event'): string
'clientEngine', 'clientEngineVersion',
'deviceName', 'deviceBrand', 'deviceModel',
'hostname', 'ip',
// premium geo (lower-cardinality only; city/isp/AS org/connection org
// are high-cardinality and intentionally fall through to Nullable(String))
// request attributes: only protocol is bounded. accept/acceptLanguage/
// queryKeys hold raw, un-normalized caller input (unbounded distinct
// values), so they stay plain Nullable(String) + bloom_filter.
'protocol',
// ip reputation verdict (placeholder): bounded enum
// (clean/low/suspicious/block)
'ipReputation',
// premium geo (lower-cardinality only; city/isp/AS org/connection org and
// postalCode/latitude/longitude are high-cardinality and intentionally fall
// through to Nullable(String))
'continentCode', 'subdivisions', 'connectionType',
'connectionUsageType', 'autonomousSystemNumber',
'timeZone', 'weatherCode',
// sdk identity
'sdk', 'sdkVersion',
// gauge replica ordinal
Expand Down Expand Up @@ -1843,6 +1852,8 @@ private function getColumnCodec(string $id): string
'autonomousSystemNumber', 'autonomousSystemOrganization',
'connectionType', 'connectionUsageType', 'connectionOrganization',
'sdk', 'sdkVersion',
// high-entropy, un-normalized request text
'accept', 'acceptLanguage', 'queryKeys',
];

if (in_array($id, $zstdColumns, true)) {
Expand Down
36 changes: 32 additions & 4 deletions src/Usage/Metric.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ class Metric extends ArrayObject
'country', 'region', 'hostname', 'ip',
// request attributes (firewall rule matching)
'protocol', 'accept', 'acceptLanguage', 'queryKeys',
// ip reputation (placeholder — populated later; bounded verdict enum:
// clean/low/suspicious/block)
'ipReputation',
// premium geo
'city', 'continentCode', 'subdivisions',
'postalCode', 'latitude', 'longitude', 'timeZone', 'weatherCode',
Expand Down Expand Up @@ -92,6 +95,7 @@ class Metric extends ArrayObject
* - teamId / teamInternalId: owning team identity
* - country / region / hostname / ip: geographic + caller origin
* - protocol / accept / acceptLanguage / queryKeys: request attributes (firewall rule matching)
* - ipReputation: IP reputation verdict (placeholder; clean/low/suspicious/block)
* - city / continentCode / subdivisions: premium geo location fields
* - postalCode / latitude / longitude / timeZone / weatherCode: premium geo location fields
* - isp / autonomousSystemNumber / autonomousSystemOrganization: premium network origin
Expand Down Expand Up @@ -658,6 +662,8 @@ public static function getEventSchema(): array
$stringColumn('accept', 1024),
$stringColumn('acceptLanguage', 256),
$stringColumn('queryKeys', 1024),
// ip reputation (placeholder — bounded verdict enum)
$stringColumn('ipReputation', 32),
// premium geo
$stringColumn('city', 256),
$stringColumn('continentCode', 2),
Expand Down Expand Up @@ -776,20 +782,42 @@ public static function getEventIndexes(): array
'teamId', 'teamInternalId',
'country', 'region', 'hostname', 'ip',
'osName', 'clientType', 'clientName', 'deviceName',
// request attributes (firewall rule matching, exact-match)
'protocol', 'accept', 'acceptLanguage', 'queryKeys',
// ip reputation (placeholder — bounded verdict enum, exact-match)
'ipReputation',
// premium geo (latitude/longitude are display-only: no exact-match
// filtering, so intentionally left unindexed)
'postalCode', 'timeZone', 'weatherCode',
];

$setIndexed = ['status', 'method', 'country', 'service', 'clientType', 'osName'];
$setIndexed = [
'status', 'method', 'country', 'service', 'clientType', 'osName',
// low-cardinality request/geo dims filtered by equality. accept/
// acceptLanguage/queryKeys are unbounded caller input, so they use
// bloom_filter (below) instead of an unlimited set(0) index.
'protocol', 'ipReputation', 'timeZone', 'weatherCode',
];

// Columns whose full length exceeds the SQL adapter's max index key
// length (768 bytes) must be indexed on a prefix; ClickHouse ignores
// the prefix and indexes the whole value.
$prefixed = [
'path' => 255,
'accept' => 255,
'queryKeys' => 255,
];
Comment thread
greptile-apps[bot] marked this conversation as resolved.

return array_map(
static function (string $col) use ($setIndexed): array {
static function (string $col) use ($setIndexed, $prefixed): array {
$entry = [
'$id' => 'index-' . $col,
'type' => 'key',
'attributes' => [$col],
'indexType' => in_array($col, $setIndexed, true) ? 'set(0)' : 'bloom_filter',
];
if ($col === 'path') {
$entry['lengths'] = [255];
if (isset($prefixed[$col])) {
$entry['lengths'] = [$prefixed[$col]];
}
return $entry;
},
Expand Down
33 changes: 33 additions & 0 deletions tests/Usage/Adapter/ClickHouseColumnTypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,39 @@ public function testHighCardinalityPremiumGeoColumns(): void
}
}

/**
* Low-cardinality request-attribute and premium-geo dims added in #36/#37
* must map to LowCardinality(Nullable(String)).
*/
public function testLowCardinalityRequestAndGeoColumns(): void
{
foreach (['protocol', 'ipReputation', 'timeZone', 'weatherCode'] as $col) {
$this->assertSame(
'LowCardinality(Nullable(String))',
$this->columnType($col),
"{$col} should be LowCardinality(Nullable(String))"
);
}
}

/**
* High-cardinality dims — un-normalized request text (accept/acceptLanguage/
* queryKeys) and premium-geo — must fall through to plain Nullable(String).
*/
public function testHighCardinalityRequestAndGeoColumns(): void
{
foreach ([
'accept', 'acceptLanguage', 'queryKeys',
'postalCode', 'latitude', 'longitude',
] as $col) {
$this->assertSame(
'Nullable(String)',
$this->columnType($col),
"{$col} should be plain Nullable(String)"
);
Comment thread
ArnabChatterjee20k marked this conversation as resolved.
}
}

/**
* SDK dims must map to LowCardinality(Nullable(String)).
*/
Expand Down
19 changes: 19 additions & 0 deletions tests/Usage/Adapter/ClickHouseSchemaTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,23 @@ public function testEventsTableSwapsBloomForSetOnLowCardinality(): void
$this->assertStringContainsString('`index-hostname` hostname TYPE bloom_filter', $ddl);
$this->assertStringContainsString('`index-resourceId` resourceId TYPE bloom_filter', $ddl);
$this->assertStringContainsString('`index-teamId` teamId TYPE bloom_filter', $ddl);

// request-attribute + premium-geo dims added for firewall/console filtering:
// bounded-cardinality equality dims get set(0); un-normalized text and
// high-cardinality dims get bloom_filter.
$this->assertStringContainsString('`index-protocol` protocol TYPE set(0)', $ddl);
$this->assertStringContainsString('`index-ipReputation` ipReputation TYPE set(0)', $ddl);
$this->assertStringContainsString('`index-timeZone` timeZone TYPE set(0)', $ddl);
$this->assertStringContainsString('`index-weatherCode` weatherCode TYPE set(0)', $ddl);

$this->assertStringContainsString('`index-accept` accept TYPE bloom_filter', $ddl);
$this->assertStringContainsString('`index-acceptLanguage` acceptLanguage TYPE bloom_filter', $ddl);
$this->assertStringContainsString('`index-queryKeys` queryKeys TYPE bloom_filter', $ddl);
$this->assertStringContainsString('`index-postalCode` postalCode TYPE bloom_filter', $ddl);

// latitude/longitude are display-only and intentionally NOT indexed.
$this->assertStringNotContainsString('`index-latitude`', $ddl);
$this->assertStringNotContainsString('`index-longitude`', $ddl);
}

public function testDailyTableMatchesPrePrSchema(): void
Expand Down Expand Up @@ -299,8 +316,10 @@ private function expectedDimAssertions(array $columns, string $type): array
'clientEngine', 'clientEngineVersion',
'deviceName', 'deviceBrand', 'deviceModel',
'hostname', 'ip',
'protocol', 'ipReputation',
'continentCode', 'subdivisions', 'connectionType',
'connectionUsageType', 'autonomousSystemNumber',
'timeZone', 'weatherCode',
'sdk', 'sdkVersion',
'ordinal',
];
Expand Down
8 changes: 8 additions & 0 deletions tests/Usage/MetricTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,16 @@ public function testEventIndexesCoverNewFilterableColumns(): void
'resourceId', 'resourceInternalId', 'teamId', 'teamInternalId',
'country', 'region', 'hostname', 'ip',
'osName', 'clientType', 'clientName', 'deviceName',
'protocol', 'accept', 'acceptLanguage', 'queryKeys',
'ipReputation',
'postalCode', 'timeZone', 'weatherCode',
] as $col) {
$this->assertContains($col, $indexed, "Event indexes missing {$col}");
}

// latitude/longitude are display-only and intentionally NOT indexed.
$this->assertNotContains('latitude', $indexed);
$this->assertNotContains('longitude', $indexed);
}

public function testGaugeIndexesCoverIdColumns(): void
Expand Down Expand Up @@ -610,6 +617,7 @@ public function testEventColumnsConstant(): void
'teamId', 'teamInternalId',
'country', 'region', 'hostname', 'ip',
'protocol', 'accept', 'acceptLanguage', 'queryKeys',
'ipReputation',
'city', 'continentCode', 'subdivisions',
'postalCode', 'latitude', 'longitude', 'timeZone', 'weatherCode',
'isp', 'autonomousSystemNumber', 'autonomousSystemOrganization',
Expand Down
Loading