-
Notifications
You must be signed in to change notification settings - Fork 0
feat(schema): add resourceType dimension to events and gauges (column + index + projection) #17
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -682,6 +682,7 @@ private function formatParamValue(mixed $value): string | |
| ['name' => 'p_by_path', 'dims' => ['path']], | ||
| ['name' => 'p_by_country', 'dims' => ['country']], | ||
| ['name' => 'p_by_service', 'dims' => ['service']], | ||
| ['name' => 'p_by_resourceType', 'dims' => ['resourceType']], | ||
| ]; | ||
|
|
||
| /** | ||
|
|
@@ -695,8 +696,10 @@ private function formatParamValue(mixed $value): string | |
| private const GAUGE_PROJECTIONS = [ | ||
| ['name' => 'p_by_service', 'dims' => ['service']], | ||
| ['name' => 'p_by_resource', 'dims' => ['resource']], | ||
| ['name' => 'p_by_resourceType', 'dims' => ['resourceType']], | ||
| ['name' => 'p_by_resourceId', 'dims' => ['resourceId']], | ||
| ['name' => 'p_by_resource_resourceId', 'dims' => ['resource', 'resourceId']], | ||
| ['name' => 'p_by_resourceType_resource', 'dims' => ['resourceType', 'resource']], | ||
| ]; | ||
|
|
||
| /** | ||
|
|
@@ -741,6 +744,7 @@ public function setup(): void | |
| ); | ||
|
|
||
| $this->ensureGaugeDimColumns(); | ||
| $this->ensureEventDimColumns(); | ||
|
|
||
| // --- Per-dim projections on the events / gauges base tables --- | ||
| $this->setLightweightMutationProjectionMode($this->getEventsTableName()); | ||
|
|
@@ -777,7 +781,7 @@ private function setLightweightMutationProjectionMode(string $baseTable): void | |
| } | ||
|
|
||
| /** | ||
| * Backfill the service / resource columns on an existing gauges table. | ||
| * Backfill late-added dim columns on an existing gauges table. | ||
| * setup() uses CREATE TABLE IF NOT EXISTS, so deployments that came up | ||
| * before these columns were added never receive them — the gauge | ||
| * projections would then fail because their SELECT references columns | ||
|
|
@@ -790,7 +794,26 @@ private function ensureGaugeDimColumns(): void | |
|
|
||
| $sql = "ALTER TABLE {$gaugesTable} " | ||
| . 'ADD COLUMN IF NOT EXISTS service LowCardinality(Nullable(String)), ' | ||
| . 'ADD COLUMN IF NOT EXISTS resource LowCardinality(Nullable(String))'; | ||
| . 'ADD COLUMN IF NOT EXISTS resource LowCardinality(Nullable(String)), ' | ||
| . 'ADD COLUMN IF NOT EXISTS resourceType LowCardinality(Nullable(String))'; | ||
|
|
||
| $this->query($sql); | ||
| } | ||
|
|
||
| /** | ||
| * Backfill late-added dim columns on an existing events table. Same | ||
| * reasoning as ensureGaugeDimColumns — CREATE TABLE IF NOT EXISTS won't | ||
| * pick up columns added to the schema after the table was first created, | ||
| * and a per-dim projection on `resourceType` cannot be materialized until | ||
| * the source column exists on the base table. | ||
| */ | ||
| private function ensureEventDimColumns(): void | ||
| { | ||
| $eventsTable = $this->escapeIdentifier($this->database) | ||
| . '.' . $this->escapeIdentifier($this->getEventsTableName()); | ||
|
|
||
| $sql = "ALTER TABLE {$eventsTable} " | ||
| . 'ADD COLUMN IF NOT EXISTS resourceType LowCardinality(Nullable(String))'; | ||
|
|
||
| $this->query($sql); | ||
|
Comment on lines
795
to
818
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This migration only adds the new ArtifactsRepro: generated migration harness
Repro: harness output showing missing resourceType index migration
|
||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The events base table gets
resourceType, but the daily events table, daily materialized view,DAILY_COLUMNS, andfindDaily()grouping still omit it. Calls that use the daily APIs with aresourceTypefilter are rejected byvalidateDailyAttributeName(), and closed-day event sums filtered byresourceTypecannot use the daily rollup because the router treats the column as not daily-safe. This leavesresourceTypeincomplete as a first-class events dimension for historical usage reads.