Repository navigation
Conversation
This reverts commit a2dd31c. Conflict resolutions against current main: - Cargo.toml files: keep quickwit-transport, quickwit-telemetry-exporters and procfs; add quickwit-telemetry back. The workspace no longer has a default-members list, so only members and workspace.dependencies change. - quickwit-cli/src/main.rs: keep parse_cli_command() and have it call the restored about_text(). - quickwit-cli/src/service.rs: keep the SIGHUP TLS reload. The service override now happens in load_node_config(), so only the telemetry loop and RunCommand/EndCommand events are restored. - Drop the datafusion-ci workflow, datafusion sandbox and metrics_distributed_tests changes: those files were removed in #6809. - Set QW_DISABLE_TELEMETRY in full-tests.yml (added in #6718), since make test-all runs RunCliCommand::execute from quickwit-cli tests. - Cargo.lock: only adds the telemetry crate and its dependencies, with no other re-resolution.
- Add a telemetry relay in distribution/telemetry-relay (Cloudflare Worker). It validates the payload and keeps only documented fields, never reads or forwards client IP addresses or headers, replaces the unsalted MD5 host hash with an HMAC keyed with a relay-only secret, and forwards one Datadog log per event. - Rewrite docs/telemetry.md: Datadog receives the data through the relay, list the events, services and features actually sent, and fix the --help example. - Make QW_DISABLE_TELEMETRY fail closed: any non-empty value other than false, no or 0 disables telemetry. Values such as "on" used to leave telemetry enabled, contrary to the docs. - Report more features from the node config: the storage backend of the default index root URI, Lambda leaf search, split cache and TLS.
Add the machine size, build info, a persistent install ID, a hashed cluster ID, more features, API request counts and a cluster summary. The relay runs behind CloudFront, which provides the country and city.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9985cfdea6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let mut redacted_metastore_uri = config.metastore_uri.clone(); | ||
| redacted_metastore_uri.redact(); |
There was a problem hiding this comment.
For PostgreSQL metastore URLs, Uri::redact() masks only the password while preserving the username, so including this redacted URI in the hash makes nodes from the same cluster report different cluster identities when they use per-node database users or rotate the user. It also contradicts the documented credential-independent identity; remove the complete userinfo component before hashing.
Useful? React with 👍 / 👎.
| pub fn running() -> Self { | ||
| TelemetryEvent::Running { | ||
| api_usage: API_REQUEST_COUNTS.take(), | ||
| cluster_summary: cluster_summary(), |
There was a problem hiding this comment.
When the ten-event queue is full—for example, after repeated UI page loads during the one-minute cooldown—running() resets all API counters here before EventsState::push_event rejects the summary event. The next report therefore permanently undercounts that interval; end_command() has the same issue at shutdown. Preserve the counts unless the summary event is successfully enqueued.
Useful? React with 👍 / 👎.
| env: | ||
| CARGO_INCREMENTAL: 0 | ||
| QW_DISABLE_TELEMETRY: 1 |
There was a problem hiding this comment.
The opt-out is added to the Rust CI workflows, but .github/workflows/ui-ci.yml still starts a real server with cargo run --features=postgres -- run ... in its e2e job without QW_DISABLE_TELEMETRY. Every UI CI run will therefore send install, lifecycle, UI, and API-usage events to the production relay and contaminate the usage data; set the same environment variable on that job.
Useful? React with 👍 / 👎.
| if config.indexer_config.enable_otlp_endpoint { | ||
| features.insert(QuickwitFeature::Otlp); | ||
| } | ||
| if config.jaeger_config.enable_endpoint { | ||
| features.insert(QuickwitFeature::Jaeger); |
There was a problem hiding this comment.
With production defaults, these flags are true even on a metastore-only or control-plane-only node, so such nodes report both otlp and jaeger despite exposing neither endpoint. serve_quickwit only constructs OTLP services when Indexer is enabled and the Jaeger service when Searcher is enabled; apply those same service checks here to avoid substantially overstating feature adoption.
Useful? React with 👍 / 👎.
| const city = headers.get(CITY_HEADER); | ||
| if (city !== null && CITY_RE.test(city)) { | ||
| location.city = city; |
There was a problem hiding this comment.
CloudFront percent-encodes non-ASCII characters in viewer-location headers, as documented by AWS, but this code stores the header verbatim. Cities such as São Paulo will consequently be recorded with encoded text such as S%C3%A3o Paulo, fragmenting location aggregation; safely percent-decode the header before validating and forwarding it.
Useful? React with 👍 / 👎.
| function parseNames(value, field) { | ||
| check(Array.isArray(value), `${field} must be a list`); | ||
| check(value.length <= MAX_NUM_NAMES, `${field} has more than ${MAX_NUM_NAMES} items`); | ||
| for (const name of value) { | ||
| check(isMatch(name, NAME_RE), `${field} contains an invalid name`); | ||
| } | ||
| return [...new Set(value)].sort(); |
There was a problem hiding this comment.
parseNames validates only the shape of each string, so arbitrary identifiers such as customer_logs or private-index are accepted and forwarded when placed in services, features, or cluster_summary.source_types. Because this relay is the privacy boundary intended to reject fields outside the documented schema, a malformed client can persist user-specific names that the Rust enums would never emit; use schema-versioned allowlists for each of these fields rather than a generic identifier regex.
Useful? React with 👍 / 👎.
dayaffe
added
area:core
The relay now lives with its infrastructure in a separate repository. Read the memory size with sysinfo, replace an invalid install ID instead of failing, and trim the tests. Leave the database user and query parameters out of the cluster hash, take the API request counts only once their event is queued, report the OTLP and Jaeger features only on the nodes that serve them, and disable telemetry in the UI CI.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e139dec72e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Ok(install_id_str) => { | ||
| if let Ok(install_id) = Uuid::parse_str(install_id_str.trim()) { | ||
| return Ok((install_id, false)); | ||
| } | ||
| } |
There was a problem hiding this comment.
If telemetry_install_id is truncated or corrupted—for example, by a crash during the non-atomic write below—this branch silently replaces it and returns created = true. The next startup therefore reports a second installation under a new UUID, breaking the promised persistent identity and contaminating install counts; treat an existing invalid file as an error and write newly created IDs atomically instead.
AGENTS.md reference: AGENTS.md:L19-L22
Useful? React with 👍 / 👎.
| if let Some((before_query, _)) = location.split_once('?') { | ||
| location = before_query; | ||
| } |
There was a problem hiding this comment.
For file-backed and S3 metastores, Quickwit encodes the polling setting as a URI fragment such as #polling_interval=30s, but this normalization strips only ? parameters. Nodes using the same cluster ID and storage location but different polling intervals therefore produce different cluster hashes, fragmenting the node and cluster counts; remove the fragment before computing the digest as well.
Useful? React with 👍 / 👎.
Description
Restores the anonymous usage telemetry removed in #6431. Nodes send it to
telemetry.quickwit.io, a relay operated by Datadog that forwards it to Datadog Logs.QW_DISABLE_TELEMETRYfail closed: any non-empty value other thanfalse,noor0disables telemetry. Values such asonused to leave it enabled.installevent, a hashed cluster ID, the ingest v1/v2, S3 flavor and docs clustering features, bucketed API request counts, and, from the control plane, the index count and the source types.docs/telemetry.mdto list everything that is sent and where it goes.Once
telemetry.quickwit.iopoints at the relay, releases 0.6.0 to 0.8.x, which still send to that address, start reporting again.How was this PR tested?
Unit tests for the telemetry crate, the CLI, the API counters and the control plane summary. Deployed the relay to a throwaway AWS account and ran a Quickwit node through it twice, with indexes, sources and requests on each counted API. All events and fields reached a test Datadog org, and the
installevent was only sent on the first run.