fix(MonitorThresholds): treat empty string as null for f64 threshold fields#1292
Open
platinummonkey wants to merge 1 commit intoDataDog:masterfrom
Open
Conversation
…fields The Datadog API returns "" instead of null for unset threshold values on some monitor types (e.g. service check monitors, composite monitors). The deserializer only guarded against null, causing a serde panic: invalid type: string "", expected f64 Apply the same null-skip guard to all six threshold fields: critical, critical_recovery, ok, unknown, warning, warning_recovery. Reproducer: call ListMonitors on an org that has service-check monitors with unset warning/critical_recovery thresholds.
platinummonkey
added a commit
to datadog-labs/pup
that referenced
this pull request
Mar 1, 2026
Implements `pup vet` per the design in discussion #132. Surfaces universally broken Datadog configurations with a single monitors API call. Checks implemented: - silent-monitors (CRITICAL): monitors with no @-mention/notification channel in their message — alerts fire into the void - stale-monitors (WARNING): monitors currently in "No Data" state — abandoned or misconfigured data source - muted-forgotten (WARNING): monitors muted indefinitely or with a silence expiry >30 days out New files: - src/ops/mod.rs + src/ops/vet.rs — check engine with shared types (Severity, Resource, Finding, VetResult) - src/commands/vet.rs — CLI presentation layer (human + agent mode) Usage: pup vet # run all checks pup vet --tags=team:platform # scope by monitor tags pup vet --check=silent-monitors # single check pup vet --severity=critical # filter output pup vet list # list available checks Also adds a [patch.crates-io] for datadog-api-client pointing at a local fix branch for a serde deserialisation crash when the Datadog API returns "" for f64 threshold fields (MonitorThresholds). Upstream fix proposed at DataDog/datadog-api-client-rust#1292. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
4 tasks
| match k.as_str() { | ||
| "critical" => { | ||
| if v.is_null() { | ||
| if v.is_null() || v.as_str() == Some("") { |
Contributor
There was a problem hiding this comment.
This code is generated so it would be replaced on the next spec change. You'd likely have to make the change to
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Datadog API occasionally returns
""(empty string) instead ofnullfor unset threshold values on certain monitor types (e.g. service check monitors, composite monitors with partial threshold configs). TheMonitorThresholdsdeserializer only guardedcriticalagainstnull, leaving empty strings to fall through toserde_json::from_value::<f64>()which panics with:This causes
list_monitorsto fail entirely for any org that has one of these monitors.Changes
src/datadogV1/model/model_monitor_thresholds.rs: Add|| v.as_str() == Some("")guard to all six threshold fields:critical,critical_recovery,ok,unknown,warning,warning_recoveryReproducer
Call
list_monitorson an org that has service-check monitors or composite monitors where optional thresholds (warning,critical_recovery, etc.) were never set via the API — the UI sometimes persists these as""rather than omitting the field.Test
The existing
MonitorThresholdsdeserialization tests should continue to pass. A new case worth adding: