Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: remove 'd' from duration values spec #674

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

tpaschalis
Copy link
Member

Fixes #599

This PR addresses a spec inconsistency. This is not a breaking change, since d was never actually supported as a valid duration unit.

I still think that the Go team had a good rationale for not adding support for it in time.ParseDuration; if it's so important for people to use higher values we could find a way to support richer expressions such as 5 * 24h in the future instead.

Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
@tpaschalis
Copy link
Member Author

cc @rfratto for visibility, I'll go ahead and merge this if there's no objections.

@rfratto
Copy link
Member

rfratto commented Apr 25, 2024

since d was never actually supported as a valid duration unit.

To be precise, Alloy/Flow mode has never supported it, but Prometheus and static mode of Grafana Agent do thanks to github.com/prometheus/common/model.Duration

Personally I find most people mean 1d to be 24h, even if sometimes a day is not exactly 24h. I'm ok with us fixing the docs since no component uses model.Duration, but we should double check that the config converters handle converting 1d from Prometheus to 24h in Alloy properly (they should, but wouldn't hurt to check).

@rfratto rfratto added backport release/v1.0 backport-to-agent PR should be backported to the agent repo. labels Apr 25, 2024
@tpaschalis
Copy link
Member Author

Thanks for the feedback! I've verified that the converters do handle days properly (1d gets converted to 24h0m0s).

I'm merging this.

@tpaschalis tpaschalis merged commit eab8cd9 into main Apr 25, 2024
15 checks passed
@tpaschalis tpaschalis deleted the remove-d-from-duration-spec branch April 25, 2024 13:31
github-actions bot pushed a commit that referenced this pull request Apr 25, 2024
Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
(cherry picked from commit eab8cd9)
rfratto pushed a commit that referenced this pull request Apr 25, 2024
Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
(cherry picked from commit eab8cd9)

Co-authored-by: Paschalis Tsilias <tpaschalis@users.noreply.github.com>
@clayton-cornell clayton-cornell added the type/docs Docs Squad label across all Grafana Labs repos label Apr 25, 2024
hainenber pushed a commit to hainenber/alloy that referenced this pull request May 1, 2024
Signed-off-by: Paschalis Tsilias <paschalist0@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport release/v1.0 backport-to-agent PR should be backported to the agent repo. frozen-due-to-age type/docs Docs Squad label across all Grafana Labs repos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type duration "d" abbrev is not respected by alloy when reading it's config
4 participants