feat(statsd source): add convert_to option for controlling timing unit conversion
#22716
Conversation
This file contains 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
This commit introduces a new boolean configuration option, convert_timers_to_seconds, in the StatsD source configuration. By default, this option is set to true (the current behavior) so that timing values in milliseconds are converted to seconds. Setting it to false disables the conversion, preserving the original millisecond values. Changes include: Adding the convert_timers_to_seconds field (with a default of true) in UDP and TCP source configurations and, where applicable, in the Unix configuration. Updating the StatsD parser to conditionally convert ms values based on the new option. Adjusting unit and integration tests to account for the new behavior. In particular, when conversion is disabled, the sample value for a timer metric reflects the original ms value multiplied by the sample rate. Updating the documentation to describe the new configuration option and its effects on downstream systems. This change improves flexibility for users integrating with sinks that expect timing values in their original units, without breaking the existing behavior by default.
…sion option This commit updates the StatsD source configuration documentation to include the new convert_timers_to_seconds field. This option controls whether incoming timing values in milliseconds are converted to seconds. By default, it is set to true (preserving current behavior); when set to false, the original millisecond values remain unchanged. Additionally, the "how_it_works.timings" section has been revised to clearly explain this behavior, ensuring that users understand the impact of this configuration on downstream metric processing.
…ption This commit adds a new changelog fragment file, "22656_statsd_timer_conversion.feature.md", to document the introduction of a configurable option in the StatsD source. With this change, users can control whether timing values in milliseconds (`ms`) are converted to seconds (`s`). By default, the conversion is enabled; however, setting `convert_timers_to_seconds` to `false` preserves the original millisecond values. This enhancement provides greater flexibility for users whose downstream systems expect timer metrics in their original units and was inspired by the discussion in GitHub Discussion [vectordotdev#22656](vectordotdev#22656). authors: devkoriel
This commit changes the implementation of `default_convert_timers_to_seconds` from a regular function to a `const fn` in the StatsD source. This update resolves compilation issues in constant contexts, ensuring that the default value for the new `convert_timers_to_seconds` configuration field is usable in const initializations. The function's behavior remains unchanged, as it continues to return `true`.
|
|
github-actions
bot
added
domain: sources
…licit `convert_to` unit This commit replaces the previous `convert_timers_to_seconds` boolean configuration with a new enum-based `convert_to` field of type `ConversionUnit`. This change allows users to explicitly specify the target unit for StatsD timing values. Key changes include: - Introduced the `ConversionUnit` enum with variants: `Seconds` (default) and `Milliseconds`. - Updated all relevant config structures (UDP, TCP, and Unix) to use `convert_to` instead of a boolean. - Updated the `Parser` logic to conditionally convert `ms` based on the `convert_to` value. - Implemented the `Configurable`, `Default`, `ToValue`, `Deserialize`, and `Serialize` traits for `ConversionUnit`. - Updated `generate_schema` to properly generate a string enum schema for the new option. - Updated docs (`*.cue`) and help text to reflect the new `convert_to` configuration. - Cleaned up old usage of `convert_timers_to_seconds` and fixed Clippy lints (`derivable_impls`, `field_reassign_with_default`, etc). This change provides users with more precise control over timing conversions while maintaining backward-compatible behavior by default. authors: devkoriel
devkoriel
changed the title
convert_to option for controlling timing unit conversion
|
@pront Thank you for your support! Any updates on this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR introduces a new configuration option to the StatsD source:
convert_to, which allows users to explicitly control how incoming timing values in milliseconds (ms) are handled.By default, StatsD timing values are emitted as distributions and converted from milliseconds to seconds. This behavior was previously hardcoded. With this change, users can choose:
convert_to = "seconds"(default): preserves current behavior —msvalues are converted tos.convert_to = "milliseconds": disables conversion — values remain inms.This is a new feature; there was no
convert_timers_to_secondsin the base branch. The addition of theConversionUnitenum makes the configuration more descriptive, future-proof, and avoids implicit behavior.Key highlights:
ConversionUnitenum with variants:Seconds(default) andMilliseconds.convert_tofield to all StatsD source configs (UdpConfig,TcpConfig,UnixConfig).ms,h,d.Configurable,ToValue,Serialize,Deserialize, andDefaultfor the enum..cueandhow_it_works) to reflect the new option.Defaultimpl in favor of#[derive(Default)]+#[default]on enum variant.field_reassign_with_default).This feature was inspired by Discussion #22656, where it was suggested to replace implicit
ms → sconversion with an explicit, configurable option.Change Type
Is this a breaking change?
How did you test this PR?
convert_to = "milliseconds"preserves original values.convert_to = "seconds"applies the expected conversion.glork,timer, etc.) reflect expected sampling behavior.cargo vdev test,cargo nextest run,cargo clippy --all-targets -- -D warnings, andmake check-all..cueupdates render as expected.Does this PR include user-facing changes?
changelog.d/per guidelines.no-changeloglabel.Checklist
cargo fmt --allcargo clippy --workspace --all-targets -- -D warningscargo nextest run --workspacemake check-alldd-rust-license-tool write).cuedocs andhow_it_worksblocks to reflect user-facing config changes.References
Please let me know if further changes are required.