feat(datadog metrics sink): V3 encoder based on v0.57.0 - #26086
feat(datadog metrics sink): V3 encoder based on v0.57.0#26086StephenWakely wants to merge 18 commits into
Conversation
… from stephen/v3_vector Ports the V2+V3 dual-write datadog_metrics sink functionality (V3 columnar protobuf series encoder using the saluki datadog-agent-metrics-v3 crate, X-Metrics-Request-ID correlation, sketch shadow support, and related config/request-builder/service changes) from stephen/v3_vector onto the v0.57.0 release tag. This is the combined diff between 2567199 (the commit stephen/v3_vector branched the v3 work from) and d9a0a45474 (tip of stephen/v3_vector), applied on top of v0.57.0.
…fault Sets `dual_write.enabled` to default to `true` so Vector's datadog_metrics sink dual-writes a sampled V3 shadow payload out of the box, without requiring explicit opt-in configuration. Set `dual_write.enabled: false` to restore the previous opt-in behavior.
Applies the newer clippy ruleset (collapsible_if let-chains, trivially_copy_pass_by_ref, missing_const_for_fn) to the code ported from stephen/v3_vector so it satisfies this codebase's current #![deny(warnings)] lints and rustfmt import ordering.
The V3 sketches intake routes do not exist: both /api/intake/metrics/v3/sketches and /api/intake/metrics/v3beta/sketches return 404. A 404 maps to DatadogApiError::ClientError, which is_retriable() treats as retriable, so every sketches flush retried indefinitely against a nonexistent endpoint and never delivered anything. With dual_write now enabled by default, any pipeline carrying distributions, histograms or timings hit this. Remove the sketches shadow path entirely rather than gating it behind another config flag, so an enabled dual_write cannot produce sketches traffic: - drop sketches_uri from ShadowBuilderConfig and SKETCHES_V3_BETA_PATH - drop the sketches_shadow encoder and sketches_is_legacy tracking from DatadogMetricsRequestBuilder - only legacy (V1/V2) series flushes advance the shadow cadence, so the sampling rate no longer drifts with the sketch/series mix Series dual-write is unchanged. Primary sketches delivery via sketches_api_version is untouched. Adds regression tests covering all three properties.
Sources that don't set a timestamp (statsd being the obvious one) leave metric.timestamp() as None, and both encoders independently fall back to Utc::now() *per metric* (encoder::encode_timestamp and encoder_v3::encode_timestamp). The V2 primary and the V3 shadow are encoded sequentially from the same batch, so any flush whose encoding straddles a second boundary gets different timestamps in each payload. Observed on a 5940-point statsd flush: V2 stamped 1556 points at second N and 4384 at N+1, while the V3 shadow -- encoded moments later -- stamped all 5940 at N+1. Since the intake correlates series on (name, timestamp, tags), 1556 points (26%) appeared as present-on-one-side-only in the comparison, with a handful of apparent field mismatches where a shifted key collided with a real series at the adjacent second. Decoding both payloads confirmed names, tags, resources and values were byte-identical; only the timestamps differed. Fill the fallback in once per flush before the shadow copy is taken, so both payloads agree and every point in a flush shares one coherent timestamp. Metrics that already carry a timestamp are untouched. Verified end to end against a mock intake: the same workload went from 1556 one-sided series to 0 across four consecutive runs.
github-actions
Bot
added
docs review on hold
![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: e4f0045da9
ℹ️ 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".
…hadow write check-events: DatadogMetricsRequestError was misclassified by 'cargo vdev check events' as a terminal component error (any event ending in *Error* MUST log at error! and increment component_errors_total). This event fires once per retry attempt though, not once per failed flush -- the generic request driver already counts the final post-retry failure via CallError. Forcing it to comply would either inflate component_errors_total by the retry count, or require breaking the per-attempt diagnostic logging. Rename to DatadogMetricsRequestFailed so the check's Error-suffix heuristic no longer applies; behavior is unchanged. validate-changelog: breaking fragments require an H1 title as their first line, unlike enhancement/fix fragments. Add one to datadog_metrics_v3_dual_write_default.breaking.md.
github-actions
Bot
added
the
docs review on hold
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95bfd36503
ℹ️ 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".
| stamp_batch_id(batch_id.as_ref(), &mut shadow_results); | ||
| stamp_sequence(&mut shadow_results); | ||
| results.extend(shadow_results); |
There was a problem hiding this comment.
When the V3 shadow cannot encode or split a batch that the legacy primary encoded successfully—for example, a V1 payload that fits its 3.2 MB compressed limit but whose V3 twin exceeds 512 KB—this appends the shadow Err to the production result stream. The error is then handled by sink.rs as DatadogMetricsEncodingError with dropped_events, incrementing component error and dropped-event telemetry even though the primary payload was delivered. Route validation-only encoding failures through separate diagnostics instead of reporting production data loss.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We want failures reported.
The V3 sketches intake routes don't exist yet: both /api/intake/metrics/v3/sketches and /api/intake/metrics/v3beta/sketches return 404, and 404 maps to a retriable ClientError, so a sink configured this way retries every sketches flush forever without ever delivering it. Mark the SketchesApiVersion::V3 variant #[serde(skip)] rather than removing it: this rejects 'sketches_api_version: v3' at config-load time with a clear 'unknown variant v3, expected v2' error, and drops v3 from the generated schema/docs entirely, while keeping the variant, get_path(), is_v3_format(), and the request builder's V3 sketches encoder branch fully implemented and directly testable. Re-enabling later is just removing the one attribute. v2 (default) and series_api_version are both unaffected. Adds tests covering: v3 is rejected with an unknown-variant error, v2 and the unset default both still parse, and the existing direct-construction test proving SketchesApiVersion::V3's own code (get_path) still works.
StephenWakely
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adaa7b6a07
ℹ️ 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".
| if host_key.as_deref() == Some(key) { | ||
| if let Some(host) = value | ||
| && !host.is_empty() | ||
| { | ||
| host_resource = Some(host); |
There was a problem hiding this comment.
When the configured host tag has multiple values, MetricTags::iter_all visits each value and this assignment leaves V3 using the last non-empty one. V2 instead calls tags.remove(...), whose into_single conversion returns None for a multivalue set, and therefore emits an empty host resource. Such metrics consequently produce different V2/V3 resources during shadow validation and change host attribution when V3 is primary; mirror V2's single-value lookup behavior here.
Useful? React with 👍 / 👎.
|
FYI if your intention is to eventually merge this with the main branch we should address all codex review comments (see policy). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d702abbda6
ℹ️ 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 shadow_results = encode_batch( | ||
| &mut shadow.encoder, | ||
| api_key, | ||
| DatadogMetricsEndpoint::Series(SeriesApiVersion::V3), | ||
| shadow_m, |
There was a problem hiding this comment.
When dual_write samples a flush, this encodes the cloned shadow metrics with ordinary RequestMetadata and appends them to the same driver stream as the primary request. Consequently, a successful shadow response causes DatadogMetricsResponse::events_sent to increment component_sent_events_total and its event-byte counter a second time, while a terminal shadow request failure causes the driver to report the cloned event count through CallError even if the primary request succeeded. This makes production delivery/drop telemetry depend on validation traffic (and can double it with shadow_every: 1); shadow requests should carry zero event metadata or use a separately classified response path while still accounting for their network bytes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
We want to include shadow requests in telemetry.
2 participants
Summary
Vector configuration
How did you test this PR?
Is this a breaking change?
Does this PR include user facing changes?
no-changeloglabel to this PR.References
Notes
@vectordotdev/vectorto reach out to us regarding this PR.pre-pushhook, please see this template.make fmtmake check-clippy(if there are failures it's possible some of them can be fixed withmake clippy-fix)make testgit merge origin masterandgit push.Cargo.lock), pleaserun
make build-licensesto regenerate the license inventory and commit the changes (if any). More details on the dd-rust-license-tool.