chore(config): model duration defaults as a typed value#1966
Conversation
Follow-up to the classifier duration fix that models `format: duration` as a first-class type instead of an `is_duration` side channel, and canonicalizes duration defaults at build time rather than reparsing a schema string on every classification. - Add a `go-duration` leaf crate that owns Go `time.ParseDuration` parsing, and delegate `saluki-config`'s `DurationString` to it so the algorithm has a single owner (was duplicated by the classifier fix). - Fold `format: duration` into `FieldType::Duration` (and a matching `ValueType::Duration` for smoke tests), replacing the `is_duration` flag. `parse_value_type` panics on an illogical duration base type. - Replace `ClassifierEntry`'s `Option<&str>` default + `is_duration` with a typed `DefaultValue` (`Missing` / `Json` / `DurationNanos`). Codegen parses duration defaults to nanoseconds via `go-duration` and fails the build if a `format: duration` default isn't a valid Go duration; the runtime check just switches on the typed default. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Binary Size Analysis (Agent Data Plane)Baseline: 4de556d · Comparison: cbd376b · diff ✅ Binary size difference within thresholdChanges by Module
Detailed Symbol Changes |
Regression Detector (Agent Data Plane)Run ID: Optimization Goals: ✅ No significant changes detectedFine details of change detection per experiment (5)Experiments configured
Bounds Checks: ✅ Passed (5)
ExplanationA change is flagged as a regression when |Δ mean %| > 5.00% in the regressing direction for its optimization goal AND SMP marks the experiment as a regression ( |
Reword doc comments flagged by check-docs: drop spaced em-dashes and avoid dictionary-rejected words (canonicalized/reparses/reimplemented). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
webern
left a comment
There was a problem hiding this comment.
LGTM on a quick pass and does what I think I need on my branch(es) 👍
Merged main's advancement, including removal of dogstatsd_pipe_name and dogstatsd_windows_pipe_security_descriptor keys from the overlay. Regenerated classifier_data.rs from the merged overlay schema. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
## Summary Follow-up to #1958 (now merged), taking the alternative design proposed in review (#1958 (comment)): model `format: duration` as a first-class type and store **parsed** duration defaults in the generated classifier data, instead of carrying an `is_duration` side channel and reparsing a schema string on every classification. The base type is modeled correctly, and an invalid duration default now breaks the build rather than misbehaving at runtime. Key changes: - **New `go-duration` leaf crate** owns Go `time.ParseDuration` parsing. `saluki-config`'s `DurationString` delegates to it, and the classifier build script uses it too — so the algorithm has a single owner instead of being reimplemented (the parser #1958 added to the classifier is removed). An upstream crate (for example `go-parse-duration`) can replace this later after vetting; keeping it in-tree for now avoids taking an unvetted external dependency. - **`FieldType::Duration`** (plus a matching `ValueType::Duration` for the config-testing smoke tests) replaces the `is_duration` flag. `parse_value_type` folds `format: duration` in and panics on an illogical base type. Smoke tests use a generic `"42s"` value so future duration keys need no bespoke `test_json`. - **Typed `DefaultValue`** (`Missing` / `Json(&'static str)` / `DurationNanos(u64)`) replaces `Option<&str>` + `is_duration` on `ClassifierEntry`. Codegen parses duration defaults to nanoseconds via `go-duration` and **fails the build** if a `format: duration` default isn't a valid Go duration. The runtime default check simply switches on the typed default; it never parses a schema default. Behavior matches #1958 for the `tls_handshake_timeout` bug (a default-valued duration is recognized as default whether sent as nanoseconds or a Go duration string), but the model is cleaner and the failure mode moves to compile time. ## Test plan - [x] `go-duration` unit tests (Go-style units, signs/fractions/zero, largest value, invalid/overflow, error messages). - [x] `saluki-config` duration tests pass against the delegated parser. - [x] Classifier unit tests updated for `DefaultValue` (per-variant `is_default_value` coverage; duration matched both as nanoseconds and as a Go duration string). - [x] `cargo check --workspace --tests` and `cargo clippy` clean; config-testing smoke tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com> 7b98c6f
Summary
Follow-up to #1958 (now merged), taking the alternative design proposed in review (#1958 (comment)): model
format: durationas a first-class type and store parsed duration defaults in the generated classifier data, instead of carrying anis_durationside channel and reparsing a schema string on every classification. The base type is modeled correctly, and an invalid duration default now breaks the build rather than misbehaving at runtime.Key changes:
go-durationleaf crate owns Gotime.ParseDurationparsing.saluki-config'sDurationStringdelegates to it, and the classifier build script uses it too — so the algorithm has a single owner instead of being reimplemented (the parser fix(config): recognize duration keys at their default in the config classifier #1958 added to the classifier is removed). An upstream crate (for examplego-parse-duration) can replace this later after vetting; keeping it in-tree for now avoids taking an unvetted external dependency.FieldType::Duration(plus a matchingValueType::Durationfor the config-testing smoke tests) replaces theis_durationflag.parse_value_typefoldsformat: durationin and panics on an illogical base type. Smoke tests use a generic"42s"value so future duration keys need no bespoketest_json.DefaultValue(Missing/Json(&'static str)/DurationNanos(u64)) replacesOption<&str>+is_durationonClassifierEntry. Codegen parses duration defaults to nanoseconds viago-durationand fails the build if aformat: durationdefault isn't a valid Go duration. The runtime default check simply switches on the typed default; it never parses a schema default.Behavior matches #1958 for the
tls_handshake_timeoutbug (a default-valued duration is recognized as default whether sent as nanoseconds or a Go duration string), but the model is cleaner and the failure mode moves to compile time.Test plan
go-durationunit tests (Go-style units, signs/fractions/zero, largest value, invalid/overflow, error messages).saluki-configduration tests pass against the delegated parser.DefaultValue(per-variantis_default_valuecoverage; duration matched both as nanoseconds and as a Go duration string).cargo check --workspace --testsandcargo clippyclean; config-testing smoke tests pass.🤖 Generated with Claude Code