Skip to content

Add sdk and sdkVersion dimension columns to usage events#25

Merged
lohanidamodar merged 1 commit into
mainfrom
CLO-4553-usage-sdk-version
Jul 13, 2026
Merged

Add sdk and sdkVersion dimension columns to usage events#25
lohanidamodar merged 1 commit into
mainfrom
CLO-4553-usage-sdk-version

Conversation

@lohanidamodar

Copy link
Copy Markdown
Contributor

Summary

Adds two new event-only dimension columns to usage events for CLO-4553:

  • sdk — the originating SDK name (e.g. web, flutter, console, cli)
  • sdkVersion — the SDK version (e.g. 14.0.0)

Both are optional strings, present in Metric::EVENT_COLUMNS and
Metric::getEventSchema() only — they never appear in the gauge columns/schema.

In the ClickHouse adapter both map to LowCardinality(Nullable(String)) with
CODEC(ZSTD(3)). Existing tables auto-materialize the columns on setup() via
the ADD COLUMN IF NOT EXISTS path; gauges are unchanged and the columns are
not added to the primary key or indexes.

Follows the exact pattern of the recent premium-geo dimensions change.

Changes

  • src/Usage/Metric.php — add sdk/sdkVersion to EVENT_COLUMNS, the schema doc block, and getEventSchema().
  • src/Usage/Adapter/ClickHouse.php — add both to the low-cardinality and ZSTD codec lists.
  • CHANGELOG.md — new 0.11.0 section.
  • Tests: extend MetricTest, ClickHouseColumnTypeTest, and ClickHouseSchemaTest.

Testing

  • phpunit tests/Usage/MetricTest.php tests/Usage/Adapter/ClickHouseColumnTypeTest.php — 55 tests pass.
  • composer lint (pint) and composer check (phpstan) pass.
  • The live ClickHouseSchemaTest requires a running ClickHouse and was not run locally.

Add two event-only SDK dimensions in Metric::EVENT_COLUMNS and
Metric::getEventSchema(): sdk (originating SDK name, e.g. web, flutter,
console, cli) and sdkVersion (e.g. 14.0.0). Both are optional strings.

In the ClickHouse adapter both map to LowCardinality(Nullable(String))
with CODEC(ZSTD(3)). Existing tables auto-materialize the columns on
setup() via the ADD COLUMN IF NOT EXISTS path; gauges are unchanged and
the columns are not added to the primary key or indexes.

Extends the Metric EVENT_COLUMNS/schema unit tests, the pure-unit
column-type mapping test, and the live ClickHouse schema test.
@greptile-apps

greptile-apps Bot commented Jul 12, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds two event-only dimension columns — sdk and sdkVersion — following the exact same pattern as the recent premium-geo dimensions addition. All six changed files are internally consistent: column ordering, max-length choices, ClickHouse type (LowCardinality(Nullable(String)) CODEC(ZSTD(3))), and schema-only placement in event (not gauge) tables match across source and test files.

  • Metric.php inserts both columns between clientEngineVersion and deviceName in EVENT_COLUMNS and getEventSchema(); GAUGE_COLUMNS and getGaugeSchema() are untouched.
  • ClickHouse.php adds both to the lowCardinality and zstdColumns lists, consistent with how connectionType/connectionUsageType were handled.
  • Tests cover the column type, DDL shape, and event-vs-gauge isolation, with the live ClickHouse schema test (ClickHouseSchemaTest) noted as requiring a running instance.

Confidence Score: 5/5

Safe to merge — additive schema change with no gauge-table or primary-key impact, and the ADD COLUMN IF NOT EXISTS migration path is already in place.

All six files change in lockstep: EVENT_COLUMNS, the event schema, both ClickHouse column-type lists, and every corresponding test are updated together. The gauge tables and existing indexes are untouched. The pattern is identical to the previously merged premium-geo dimensions and introduces no new logic paths.

No files require special attention. The live ClickHouseSchemaTest was not run locally per the PR description, but its assertions are straightforward DDL string checks that follow the same form as existing passing tests.

Important Files Changed

Filename Overview
src/Usage/Metric.php Adds sdk/sdkVersion to EVENT_COLUMNS and getEventSchema() between clientEngineVersion and deviceName; ordering and max-length choices are consistent with existing pairs.
src/Usage/Adapter/ClickHouse.php Adds both columns to the lowCardinality and zstdColumns lists, matching the pattern established for the premium-geo dimensions; gauge columns are untouched.
tests/Usage/MetricTest.php New test verifies sdk/sdkVersion are in EVENT_COLUMNS and event schema, are optional strings, and are absent from GAUGE_COLUMNS and gauge schema.
tests/Usage/Adapter/ClickHouseColumnTypeTest.php Adds a focused test confirming both columns resolve to LowCardinality(Nullable(String)).
tests/Usage/Adapter/ClickHouseSchemaTest.php DDL assertions confirm LowCardinality+ZSTD(3) for both columns; expectedDimAssertions updated so schema diff tests treat them as low-cardinality.
CHANGELOG.md Adds a 0.11.0 section documenting the new event-only dimensions, their ClickHouse type, and the ADD COLUMN migration path.

Reviews (1): Last reviewed commit: "Add sdk and sdkVersion dimension columns..." | Re-trigger Greptile

@lohanidamodar lohanidamodar merged commit 3d2914a into main Jul 13, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant