feat(config): Accept durations given in days (e.g. "7d")

[srebhan]

• Updated associated README.md.
• Wrote appropriate unit tests.
• Pull request title or commits are in conventional commit format

This PR allows to specify duration in days. This is e.g. helpful for logfile rotation or other long-running tasks or intervals.

[powersj]

I have mixed feelings on this.

On one hand, I acknowledge that it is nice for some config options. On the other hand it has never worked before now, we produce an error currently, and it is not a valid option with time.Duration and I do not like having two different behaviors.

I will approve, but just note the above.

edit: I restarted mac test as it seemed to have timed out.

[telegraf-tiger]

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

[Hipska]

I think this could be much simpler by using fmt.Scanf?

On one hand, I acknowledge that it is nice for some config options. On the other hand it has never worked before now, we produce an error currently, and it is not a valid option with time.Duration and I do not like having two different behaviors.

It was discussed on the maintainers meeting of 2022-12-20. Granted, you weren't there, but my idea was to support this for now, but with a deprecation warning given the following facts:

• Never worked, started to give errors in 1.22.0 (May 2022)
• Suggested as default value (0d) until 1.22.1 (April 2022)

So my idea is to remove this 'functionality' again after the deprecation and moving to new version with breaking changes..

[Hipska]

I don't think these formats should be supported?

[srebhan]

Why? Golang's ParseDuration supports them, so why shouldn't we?

[Hipska]

It makes it more complex and was only intended to be a temporary workaround.. Or do you think differently?

[srebhan]

Yes I do think differently as I do not think this is in any way more complex than the other features. If the code change is too complex in your view, we should completely drop the feature as it won't become any simpler...

[Hipska]

Okay, never mind then.

But do add depreciation warnings.

[Hipska]

Or aren't we going to deprecate it?

[srebhan]

I do not see what we do deprecate.... 0d is explicitly supported in the code and the remaining never worked. So either we add this or leave the stuff with an (overdue) error...

[Hipska]

The whole idea was initially to support it, but immediately deprecate it so it can be removed in next major version.

Its fine for me to not deprecate it and keep it working, but then the docs need to be updated.

[Hipska]

Please update documentation or add deprecation warning.

[powersj]

So in the past, duration with days would not error, but effectively would not be applied. I made duration with days an error, a few releases ago. This PR would allow them to actually work. I do not see us adding support and then immediately deprecating something that is already throwing an error.

Approving this and adding it is effectively a new feature to allow day based timestamps, and while it is not officially supported upstream is a convienince thing.

[Hipska]

@powersj you merged, but documentation is still not updated..