Skip to content

geoprobe: add ClickHouse persistence to geoprobe-target#3434

Open
nikw9944 wants to merge 9 commits intomainfrom
nikw9944/doublezero-3405
Open

geoprobe: add ClickHouse persistence to geoprobe-target#3434
nikw9944 wants to merge 9 commits intomainfrom
nikw9944/doublezero-3405

Conversation

@nikw9944
Copy link
Copy Markdown
Contributor

@nikw9944 nikw9944 commented Apr 2, 2026

Resolves: #3405

Summary of Changes

  • Add optional ClickHouse writing to geoprobe-target — when CLICKHOUSE_ADDR is set, received LocationOffset measurements are buffered and batch-inserted into a location_offsets table with signature verification results, raw offset bytes, and structured reference arrays
  • Use goose for schema migrations following the shredder pattern
  • Extend TestE2E_GeoprobeDiscovery with a ClickHouse testcontainer that verifies offsets land in the table with valid signatures

RFC: RFC-16 Geolocation Verification

Diff Breakdown

Category Files Lines (+/-) Net
Core logic 3 +312 / -4 +308
Scaffolding 2 +34 / -0 +34
Tests 2 +186 / -5 +181
Config/build 2 +92 / -72 +20
Other 1 +0 / -1 -1

Most new code is core logic (writer + migrations) and E2E verification.

Key files (click to expand)
  • controlplane/telemetry/internal/geoprobe/clickhouse.go — ClickhouseConfig (env var reader), connection factory, OffsetRow conversion, buffered ClickhouseWriter with 10s flush ticker
  • e2e/geoprobe_test.go — ClickHouse testcontainer helper, startGeoprobeTarget now accepts optional CH config, verifyClickhouseOffsets polls the table for valid rows
  • controlplane/telemetry/internal/geoprobe/clickhouse_test.go — unit tests for OffsetRowFromLocationOffset (including TotalRttNs accumulation) and writer buffer behavior
  • controlplane/telemetry/internal/geoprobe/migrations.go — goose migration runner with embedded SQL and slog-based logger
  • controlplane/telemetry/cmd/geoprobe-target/main.go — optional CH init from env, writer threaded through to handleOffset
  • controlplane/telemetry/db/clickhouse/migrations/20260401000001_location_offsets.sql — CREATE TABLE with MergeTree, 90-day TTL, structured reference arrays

Testing Verification

  • Unit tests for OffsetRowFromLocationOffset verify field mapping, TotalRttNs accumulation across references, and raw offset hex encoding
  • Unit test for ClickhouseWriter.Record confirms buffer accumulation without a real ClickHouse connection
  • All existing geoprobe unit tests continue to pass (pinger, coordinator, offset serialization)
  • E2E test spins up a real ClickHouse container and asserts: row count > 0, all signatures valid, total_rtt_ns > 0
  • make go-lint and make go-build pass clean

@nikw9944 nikw9944 force-pushed the nikw9944/doublezero-3405 branch from 70594de to de03678 Compare April 2, 2026 14:05
@ben-dz ben-dz self-requested a review April 2, 2026 15:37
for {
select {
case <-ctx.Done():
w.flush(context.Background())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Clickhouse is unreachable, won't this hang indefinitely with the context.Background()? It should probably have some kind of timer, or just not flush the last 10s if we're closing the program?

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.

Geolocation: geoprobe-target: write LocationOffsets to clickhouse

2 participants