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

Add date offset option to date processor #6886

Merged
merged 4 commits into from
Jan 14, 2020
Merged

Add date offset option to date processor #6886

merged 4 commits into from
Jan 14, 2020

Conversation

danielnelson
Copy link
Contributor

closes: #6877

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jan 9, 2020
@danielnelson danielnelson added this to the 1.14.0 milestone Jan 9, 2020
@danielnelson
Copy link
Contributor Author

Uh oh, it appears we have been setting the tag based on the local system time instead of using UTC to match the metric timestamps. I will fix this but it will not be fully backwards compatible.

@Crashtein
Copy link

Uh oh, it appears we have been setting the tag based on the local system time instead of using UTC to match the metric timestamps. I will fix this but it will not be fully backwards compatible.

Are you sure it is correct? Most people use this plugin to put tag offset appropiate to their timezone.
My machines are time synced and timezone has been set. Also there is a seasonal time change +/-1h in my country. When I put my point into DB through telegraf processors plugin it reads the same time stample as it has in origin machine.
So assume I want an offset -6.00h to change tag at 6:00AM local time. You want to base tag on UTC time, so finally the offset is UTC time + offset. In described case it will be -5.00h. Am I correct? Could you explain your point of view?

@danielnelson
Copy link
Contributor Author

I believe your calculation is correct. My line of thinking here is that metrics are always reported in UTC, and adding a tag with a 0 offset should match the timestamp in the database, no matter the timezone of the system who collected it.

Most people use this plugin to put tag offset appropiate to their timezone.

I don't have a good way to judge this. I suspect the primary use of this plugin is to insert day of the week or month of the year, and it is possible that many users haven't noticed the discrepancy. This would be a change of behavior though, so at the very least we should add this to the release notes.

@Crashtein
Copy link

Crashtein commented Jan 10, 2020

Ok, now I understand it. One more thing I can add to this discussion is a RTC time - reference time which may be used on linux with i.e. timedatectl. It lets you set hardware time to UTC or local. You may also consider to put an information about that and let the user choose the best solution for use.

@danielnelson
Copy link
Contributor Author

Rebased on master due to CHANGLOG.md conflict.

Added new timezone option to allow control over the time zone used. Added release note about breaking change.

@danielnelson danielnelson merged commit e8c4efb into master Jan 14, 2020
@danielnelson danielnelson deleted the date-offset branch January 14, 2020 23:16
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updated code for processors.date with offsets
3 participants