Skip to content

telemetry: geoprobe-target clickhouse fixes - #3533

Merged
nikw9944 merged 4 commits into
mainfrom
nikw/geoprobe-target-ch-fix
Apr 16, 2026
Merged

telemetry: geoprobe-target clickhouse fixes#3533
nikw9944 merged 4 commits into
mainfrom
nikw/geoprobe-target-ch-fix

Conversation

@nikw9944

@nikw9944 nikw9944 commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Strip https:// and http:// scheme prefixes from CLICKHOUSE_ADDR — the clickhouse-go driver expects plain host:port
  • Use HTTP protocol for both migration and writer connections (port 8443 is ClickHouse's HTTPS interface, not the native TCP protocol)
  • Replace fatal startup errors with lazy connection retry — the writer now connects on the first tick and retries every 10s on failure instead of crashing the process
  • Cap the in-memory row buffer at 10k while disconnected; close and reconnect on flush errors

Testing Verification

  • TestClickhouseConfigFromEnv: verifies scheme stripping for plain, https://, and http:// addresses
  • TestClickhouseWriterRecordBuffers: verifies basic buffering via the new constructor
  • TestClickhouseWriterRecordBufferCap: verifies buffer does not exceed 10k rows
  • Deployed to ams-mn-gm1 and confirmed clickhouse is working, and also that probes are received successfully without clickhouse configured

The clickhouse-go driver expects a plain host:port address but
config.env values sometimes include an https:// prefix, causing
"too many colons in address" errors. Match the controller's existing
behavior by stripping http:// and https:// prefixes.
@nikw9944
nikw9944 force-pushed the nikw/geoprobe-target-ch-fix branch from 4300ef3 to 71107f6 Compare April 15, 2026 15:03
RunMigrations creates its own clickhouse connection that was also
missing Protocol: clickhouse.HTTP, causing the same handshake error
on port 8443.
ClickhouseWriter now owns its connection lifecycle: it takes a config
instead of a pre-established connection, and connects lazily on the
first tick. If connection or migration fails, it logs an error and
retries on the next 10s tick instead of crashing the process.

Buffered rows are capped at 10k to bound memory while disconnected.
Flush errors close the connection so the next tick triggers reconnect.
@nikw9944 nikw9944 changed the title telemetry: strip URL scheme prefix from CLICKHOUSE_ADDR in geoprobe telemetry: fix geoprobe-target clickhouse connection Apr 15, 2026
@nikw9944 nikw9944 changed the title telemetry: fix geoprobe-target clickhouse connection telemetry: geoprobe-target clickhouse fixes Apr 15, 2026
@nikw9944
nikw9944 enabled auto-merge (squash) April 15, 2026 17:35
@nikw9944
nikw9944 merged commit d23d82f into main Apr 16, 2026
33 checks passed
@nikw9944
nikw9944 deleted the nikw/geoprobe-target-ch-fix branch April 16, 2026 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants