Skip to content

fix(config): normalize duration defaults to nanoseconds in classifier codegen [alt approach]#1959

Closed
jszwedko wants to merge 1 commit into
mainfrom
jszwedko/fix-config-classifier-duration-codegen
Closed

fix(config): normalize duration defaults to nanoseconds in classifier codegen [alt approach]#1959
jszwedko wants to merge 1 commit into
mainfrom
jszwedko/fix-config-classifier-duration-codegen

Conversation

@jszwedko

@jszwedko jszwedko commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Same fix as #1958, different implementation. The nightly integration suite started failing because ADP emitted a spurious Unsupported configuration key warning for tls_handshake_timeout. The root cause: the classifier stores schema defaults as JSON literals and compares them against Agent-supplied values with exact equality — but duration keys don't survive that comparison because the schema expresses their default as a Go duration string (10s) while the Agent transmits durations as integer nanoseconds (10000000000). This approach fixes it by normalizing duration defaults to nanoseconds at codegen time, so the generated classifier entry for a duration key holds an integer literal that directly matches what the Agent sends. The runtime comparison stays a plain equality check with no added logic.

Companion to #1958 (runtime duration-aware compare) for design comparison.

This PR (codegen) #1958 (runtime)
Parser lives in schema_gen.rs (build-time) classifier.rs (runtime)
ClassifierEntry changes None Adds is_duration: bool
Generated default 10000000000 (ns integer) "10s" (Go string, unchanged)
Readability of generated data Opaque integers Human-readable strings
Files changed 3 (schema_gen + classifier tests + generated) 5 (schema_gen, classifier_gen, mod.rs, classifier.rs, generated)

Test plan

  • New unit tests in schema_gen.rs: Go duration parser covers all common forms (10s, 500ms, 1h30m0s, 1.5h, 250µs, bare 0, negative, and malformed input).
  • New unit tests in classifier.rs: duration key sent as nanoseconds is recognized as its default; non-default nanosecond value is not.
  • Verified adp-config-check-no-warn and adp-config-stream pass locally against agent-dev:master-py3-full.

🤖 Generated with Claude Code

… codegen

The config classifier compares a key's incoming value against its schema
default using exact JSON equality. Duration keys broke this: the schema
default is a Go duration string (e.g. `10s`) while the Datadog Agent
transmits durations over the config stream as integer nanoseconds, so a
value at its default (e.g. `tls_handshake_timeout` = 10000000000) never
matched and was flagged as an override, emitting a spurious "Unsupported
configuration key" warning.

Normalize `format: duration` defaults to integer nanoseconds at codegen
time so the generated classifier default is stored in the same encoding
the Agent sends. The runtime comparison stays a plain equality check.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@datadog-official

This comment has been minimized.

@pr-commenter

pr-commenter Bot commented Jul 1, 2026

Copy link
Copy Markdown

Binary Size Analysis (Agent Data Plane)

Baseline: 4b4f85a · Comparison: a700cae · diff
Analysis Configuration: stripped binaries · Pass/Fail Threshold: +5%
Sizes: 40.85 MiB (baseline) vs 40.85 MiB (comparison)
Size Change: -80 B (-0.00%)

✅ Binary size difference within threshold

Changes by Module
Module File Size Symbols
anon.8235e1dce8eb18556833a2e87e74b44e.88.llvm.14815883669018479301 -6.75 KiB 1
anon.8235e1dce8eb18556833a2e87e74b44e.88.llvm.5719763036108644332 +6.74 KiB 1
anon.9d306878aa7d195117b4275f9f7253d4.5.llvm.15777899698899849513 +308 B 1
anon.9d306878aa7d195117b4275f9f7253d4.5.llvm.16258270271217397072 -308 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.46.llvm.14815883669018479301 -154 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.46.llvm.5719763036108644332 +153 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.17.llvm.14815883669018479301 -153 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.17.llvm.5719763036108644332 +152 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.19.llvm.14815883669018479301 -151 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.48.llvm.14815883669018479301 -151 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.50.llvm.14815883669018479301 -151 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.19.llvm.5719763036108644332 +150 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.48.llvm.5719763036108644332 +150 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.50.llvm.5719763036108644332 +150 B 1
anon.9d306878aa7d195117b4275f9f7253d4.10.llvm.15777899698899849513 +147 B 1
anon.9d306878aa7d195117b4275f9f7253d4.10.llvm.16258270271217397072 -147 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.79.llvm.14815883669018479301 -145 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.79.llvm.5719763036108644332 +144 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.81.llvm.14815883669018479301 -144 B 1
anon.8235e1dce8eb18556833a2e87e74b44e.81.llvm.5719763036108644332 +143 B 1
Detailed Symbol Changes
    FILE SIZE        VM SIZE    
 --------------  -------------- 
  [NEW] +6.74Ki  [NEW] +6.66Ki    anon.8235e1dce8eb18556833a2e87e74b44e.88.llvm.5719763036108644332
  [NEW]    +308  [NEW]    +218    anon.9d306878aa7d195117b4275f9f7253d4.5.llvm.15777899698899849513
  [NEW]    +153  [NEW]     +63    anon.8235e1dce8eb18556833a2e87e74b44e.46.llvm.5719763036108644332
  [NEW]    +152  [NEW]     +62    anon.8235e1dce8eb18556833a2e87e74b44e.17.llvm.5719763036108644332
  [NEW]    +150  [NEW]     +60    anon.8235e1dce8eb18556833a2e87e74b44e.19.llvm.5719763036108644332
  [NEW]    +150  [NEW]     +60    anon.8235e1dce8eb18556833a2e87e74b44e.48.llvm.5719763036108644332
  [NEW]    +150  [NEW]     +60    anon.8235e1dce8eb18556833a2e87e74b44e.50.llvm.5719763036108644332
  [NEW]    +147  [NEW]     +56    anon.9d306878aa7d195117b4275f9f7253d4.10.llvm.15777899698899849513
  [NEW]    +144  [NEW]     +54    anon.8235e1dce8eb18556833a2e87e74b44e.79.llvm.5719763036108644332
  [NEW]    +143  [NEW]     +53    anon.8235e1dce8eb18556833a2e87e74b44e.81.llvm.5719763036108644332
  -0.3%     -72  [ = ]       0    [168 Others]
  [DEL]    -144  [DEL]     -53    anon.8235e1dce8eb18556833a2e87e74b44e.81.llvm.14815883669018479301
  [DEL]    -145  [DEL]     -54    anon.8235e1dce8eb18556833a2e87e74b44e.79.llvm.14815883669018479301
  [DEL]    -147  [DEL]     -56    anon.9d306878aa7d195117b4275f9f7253d4.10.llvm.16258270271217397072
  [DEL]    -151  [DEL]     -60    anon.8235e1dce8eb18556833a2e87e74b44e.19.llvm.14815883669018479301
  [DEL]    -151  [DEL]     -60    anon.8235e1dce8eb18556833a2e87e74b44e.48.llvm.14815883669018479301
  [DEL]    -151  [DEL]     -60    anon.8235e1dce8eb18556833a2e87e74b44e.50.llvm.14815883669018479301
  [DEL]    -153  [DEL]     -62    anon.8235e1dce8eb18556833a2e87e74b44e.17.llvm.14815883669018479301
  [DEL]    -154  [DEL]     -63    anon.8235e1dce8eb18556833a2e87e74b44e.46.llvm.14815883669018479301
  [DEL]    -308  [DEL]    -218    anon.9d306878aa7d195117b4275f9f7253d4.5.llvm.16258270271217397072
  [DEL] -6.75Ki  [DEL] -6.66Ki    anon.8235e1dce8eb18556833a2e87e74b44e.88.llvm.14815883669018479301
  -0.0%     -80  [ = ]       0    TOTAL

/// Supports the unit suffixes Go's `time.ParseDuration` accepts (`ns`, `us`/`µs`/`μs`, `ms`, `s`,
/// `m`, `h`), an optional leading sign, and fractional components. Returns `None` if the string is
/// not a well-formed duration.
fn parse_go_duration_nanos(input: &str) -> Option<i128> {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be refactored out to share the implementation from saluki-config.

@pr-commenter

pr-commenter Bot commented Jul 1, 2026

Copy link
Copy Markdown

Regression Detector (Agent Data Plane)

Run ID: 7ba1e908-228c-462f-aab0-b38a05e3f427
Baseline: 4b4f85ac · Comparison: a700cae1 · diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment (5)

Experiments configured erratic: true are tagged (ignored) and skipped when determining which experiments regressed or improved. Experiments which are detected as erratic at runtime are tagged (erratic) to flag that the run's sample dispersion was high, but their regression / improvement signal still counts.

experiment goal Δ mean % links
quality_gates_rss_idle memory ⚪ +0.21 metrics profiles logs
quality_gates_rss_dsd_medium memory ⚪ +0.13 metrics profiles logs
quality_gates_rss_dsd_low memory ⚪ +0.06 metrics profiles logs
quality_gates_rss_dsd_heavy memory ⚪ -0.22 metrics profiles logs
quality_gates_rss_dsd_ultraheavy memory ⚪ -0.34 metrics profiles logs
Bounds Checks: ✅ Passed (5)
experiment check replicates observed links
quality_gates_rss_dsd_heavy memory_usage 10/10 ✅ 136 MiB ≤ 140 MiB metrics profiles logs
quality_gates_rss_dsd_low memory_usage 10/10 ✅ 42.6 MiB ≤ 50 MiB metrics profiles logs
quality_gates_rss_dsd_medium memory_usage 10/10 ✅ 65.2 MiB ≤ 75 MiB metrics profiles logs
quality_gates_rss_dsd_ultraheavy memory_usage 10/10 ✅ 191 MiB ≤ 200 MiB metrics profiles logs
quality_gates_rss_idle memory_usage 10/10 ✅ 28.4 MiB ≤ 40 MiB metrics profiles logs
Explanation

A 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 (is_regression: true). Improvements use the matching criteria for the improving direction. Experiments configured erratic: true (tagged (ignored)) are skipped outright; experiments detected as erratic at runtime (tagged (erratic)) still count, since that flag describes sample dispersion rather than directional certainty. The Δ mean % cell is colored accordingly: 🟢 = improvement, 🔴 = regression, ⚪ = neutral. Reduction in CPU or memory is an improvement; reduction in ingress throughput is a regression.

@webern webern left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is the inferior approach because it fixes the problem downstream of where the problem exists. While it's true that, currently, this may be the only downstream effect of the underspecified schema typing, we can't rely on that always being true and I think we should instead fix it at the source, more along the lines of #1958 (but I think that could be done differently and will comment over there next)

@jszwedko

jszwedko commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in-lieu of #1958

@jszwedko jszwedko closed this Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants