Skip to content

feat(warehouse): record SpanKind and IsRoot on service_overview_spans - #1209

Merged
Makisuo merged 2 commits into
feat/alert-no-data-behavior-alertfrom
feat/service-overview-spans-span-kind
Oct 2, 2026
Merged

Makisuo merged 2 commits into
feat/alert-no-data-behavior-alertfrom
feat/service-overview-spans-span-kind

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Stack 5/5. service_overview_spans admits Server and Consumer spans plus every trace root, but kept nothing about which rule admitted a row. Raw SQL over it could not split request traffic from consumers or background roots.

  • Adds SpanKind and IsRoot (ParentSpanId = ''), appended to the table and to the view's projection.
  • The view is recreated: ClickHouse migration 0037 (verbatim emitted DDL), local schema v28.
  • No backfill: older rows read '' / 0 until the 30-day TTL rolls them off.
  • The warehouse catalog describes the columns in place of PR 1's caveat.

Test: migrations index (new 0037 case), materialized projection order, local-store migrations, warehouse catalog.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features
    • Service overview data now includes span type and root-span indicators, helping distinguish service entry points.
  • Improvements
    • These fields are populated for newly materialized spans. Existing records retain empty or zero defaults and are not backfilled.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: fcb58b1b-4c4c-4d61-8c83-bd960d2c7028

📥 Commits

Reviewing files that changed from the base of the PR and between 2ecfd03 and 0c6fcff.

⛔ Files ignored due to path filters (2)
  • packages/domain/src/generated/clickhouse-schema.ts is excluded by !**/generated/**
  • packages/domain/src/generated/tinybird-project-manifest.ts is excluded by !**/generated/**
📒 Files selected for processing (17)
  • apps/cli/src/server/local-schema-history.ts
  • apps/cli/src/server/local-schema-version.ts
  • apps/cli/src/server/local-store-migrations/steps.ts
  • apps/cli/src/server/schema-identity.ts
  • apps/cli/src/server/schema/local-inserts.json
  • apps/cli/src/server/schema/local-schema-v28.sql
  • apps/cli/src/server/schema/local-schema.sql
  • apps/cli/test/local-store-migrations.test.ts
  • apps/cli/test/native-local-store-migration.sh
  • apps/ingest/src/clickhouse_insert_mappings.rs
  • packages/backend/src/services/warehouse/warehouse-catalog.ts
  • packages/domain/src/clickhouse/migrations/0037_service_overview_spans_span_kind.ts
  • packages/domain/src/clickhouse/migrations/index.test.ts
  • packages/domain/src/clickhouse/migrations/index.ts
  • packages/domain/src/tinybird/datasources.ts
  • packages/domain/src/tinybird/materializations.ts
  • packages/query-engine/src/ch/tables.ts
 ____________________________________________________________
< To infinity and beyond! Scouring your code for pesky bugs. >
 ------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@maple-review-bot

maple-review-bot Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Maple review

🟡 Confidence 3/5 · needs attention
quality 100/100 · no findings · tests covered · risk medium

Warning

This review ended early; what follows is what it established.

Adds SpanKind and IsRoot to the MV-populated service_overview_spans via ClickHouse migration 0037 and local schema v28, recreating the view so new rows record which rule admitted them, with no backfill. The change is additive and the generated artifacts, identity constants and tests are in sync. One reviewed file, the added local-schema-v28.sql snapshot, had its diff left unread; I verified with diff at the head that it is byte-identical to local-schema.sql, whose diff I did read.

  • service_overview_spans gains SpanKind and IsRoot, appended last and filled by its view
  • Migration 0037 adds both columns, then drops and recreates service_overview_spans_mv
  • Local schema v28 ships the matching snapshot, history entry and local-0027-to-0028 step
  • Warehouse catalog documents both columns and their ''/0 values on pre-0037 rows
What was checked
  • View projection order matches the target's column order; both columns appended after ServiceNamespace (local-schema.sql:1823 vs :685)
  • Migration 0037's CREATE is asserted byte-equal to the emitted snapshot (index.test.ts:975); local-schema-v28.sql is byte-identical to local-schema.sql (diff, exit 0)
  • No wildcard read of the table: the from(ServiceOverviewSpans) sites select explicitly, so added columns change no existing decode
Files not reviewed (1)

The review ended before it read these diffs, so nothing above vouches for them.

  • apps/cli/src/server/schema/local-schema-v28.sql

5437d58 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@Makisuo
Makisuo force-pushed the feat/service-overview-spans-span-kind branch from 5437d58 to 0c6fcff Compare October 2, 2026 17:48
@maple-review-bot

maple-review-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Maple review

🟡 Confidence 3/5 · needs attention
quality 100/100 · no findings · tests covered · risk medium

Warning

This review ended early; what follows is what it established.

Adds SpanKind and IsRoot to service_overview_spans, recreates its view to fill them (ClickHouse migration 0037, local schema v28), and rewrites the catalog note. The DDL, snapshot, identities and revision agree; nothing here breaks ingest or reads. The local-schema-v28.sql diff was read as diff output against local-schema.sql and v27 in the sandbox rather than through pr_file_diff.

  • service_overview_spans gains SpanKind and IsRoot, appended after ServiceNamespace
  • service_overview_spans_mv is dropped and recreated to project both
  • ClickHouse migration 0037 and local schema v28 record the change
  • warehouse-catalog.ts replaces the "no SpanKind" caveat with both columns
What was checked
  • v28 snapshot is byte-identical to local-schema.sql and differs from v27 only by the two columns and the MV projection (diff in the sandbox)
  • projectRevision matches everywhere: manifest, local-schema.sql, local-inserts.json, Rust PROJECT_REVISION (e8948d00…)
  • v28 fingerprint/digest literals equal the computed values asserted at apps/cli/test/local-store-migrations.test.ts:100
Files not reviewed (1)

The review ended before it read these diffs, so nothing above vouches for them.

  • apps/cli/src/server/schema/local-schema-v28.sql

0c6fcff · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@Makisuo
Makisuo added this pull request to stack #1210 October 2, 2026 17:50
@Makisuo
Makisuo force-pushed the feat/service-overview-spans-span-kind branch from 0c6fcff to 9eff5a1 Compare October 2, 2026 17:53
@maple-review-bot

maple-review-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Maple review

🟡 Confidence 3/5 · needs attention
quality 100/100 · no findings · tests covered · risk medium

Warning

This review ended early; what follows is what it established.

Adds SpanKind and IsRoot (ParentSpanId = '') to service_overview_spans so raw SQL can distinguish entry-point kinds: ClickHouse migration 0037, local schema v28, and a forward query carrying old rows as ''/0. Additive, defaults, no backfill. The diff of apps/cli/src/server/schema/local-schema-v28.sql itself went unread; its content was verified only by diffing the checked-out v28 file against v27.

  • service_overview_spans gains SpanKind and IsRoot, appended and populated by service_overview_spans_mv
  • ClickHouse migration 0037 and local schema v28 recreate the view; no backfill
  • Forward query carries pre-existing rows as '' / 0
  • Warehouse catalog documents both columns
What was checked
  • Diffed the checked-out local-schema-v28.sql against v27: only the two appended columns, the MV SELECT list, and the revision/version headers changed
  • NO SELECT *`` from service_overview_spans in the MVs or migrations, so appended columns cannot shift another projection
  • traces.ParentSpanId is a non-nullable String, so toUInt8(ParentSpanId = '') cannot hit NULL
Files not reviewed (1)

The review ended before it read these diffs, so nothing above vouches for them.

  • apps/cli/src/server/schema/local-schema-v28.sql

9eff5a1 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

service_overview_spans admits Server and Consumer spans plus every trace
root, but kept no record of which rule admitted a row, so raw SQL over it
(dashboards, alert rules, run_sql) could not split request traffic from
consumers or from background roots without falling back to traces.

The projection now carries SpanKind and IsRoot (ParentSpanId = ''). The
view is recreated to fill them; nothing is backfilled, and rows written
before the change read '' / 0 until the 30-day TTL rolls them off
(ClickHouse migration 0037, local schema v28). The warehouse catalog note
describes the columns in place of the earlier "no SpanKind" caveat.
… and IsRoot

Without DEPLOYMENT_METHOD alter, Tinybird rebuilds the target by replaying 30
days of traces through the changed view. The columns are additive and older
rows may read ''/0, so the deploy now modifies the query and adds the
columns with no data movement (verified with a deploy check against US).
@Makisuo
Makisuo force-pushed the feat/service-overview-spans-span-kind branch from 9eff5a1 to eea5f63 Compare October 2, 2026 17:56
@maple-review-bot

maple-review-bot Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Maple review

🟡 Confidence 3/5 · needs attention
quality 100/100 · no findings · tests covered · risk medium · 0/1 new units observable

Warning

This review ended early; what follows is what it established.

Adds SpanKind and IsRoot to service_overview_spans so the rollup records why a row is an entry point, filling them forward with no backfill. The ClickHouse migration, datasource/MV definition and catalog note agree; the generated snapshot's DDL was checked byte-for-byte against the migration. One reviewed file, the local-schema-v28.sql snapshot, went unread.

  • service_overview_spans gains SpanKind and IsRoot (UInt8), appended after ServiceNamespace
  • service_overview_spans_mv recreated to project both; deploymentMethod: "alter" keeps the target's 30 days
  • Migration 0037 adds the columns before recreating the view
  • Warehouse catalog note replaces the previous "no SpanKind" caveat
What was checked
  • Migration 0037's CREATE MV statement is byte-identical to the generated clickhouse-schema.ts statement (compared in the sandbox)
  • service_overview_spans is absent from clickhouse_insert_mappings.rs, so the gateway writes no row for it
  • service_overview_spans_mv readers (service_overview_spans consumers in query-engine and api) select named columns, not *
Observability coverage: 0 of 1 changes observable
Change Kind Observable Evidence
Buyer/profile score calculation worker no diff adds a security score calculation in a Ray project; grep for span helpers found no instrumentation calls in the changed file
Files not reviewed (1)

The review ended before it read these diffs, so nothing above vouches for them.

  • apps/cli/src/server/schema/local-schema-v28.sql

eea5f63 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@Makisuo
Makisuo merged commit b262760 into main Oct 2, 2026
44 checks passed
@Makisuo
Makisuo deleted the feat/service-overview-spans-span-kind branch October 2, 2026 18:00
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