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

Adds an 'output_precision' configuration parameter #2139

Closed

Conversation

tjmcs
Copy link

@tjmcs tjmcs commented Dec 8, 2016

Resolves #2124

The changes in this pull request add a new output_precision configuration parameter to the output plugins that utilize the Serializer interface to send influx, graphite, or json formatted data to various outputs (amqp, file, kafka, mqtt, nats, and nsq).

The format used to specify this precision value is the same format that is used to specify the precision of the timestamps used with the metrics collected by the agent (values like "1ns", "1us", "1ms" and "1s" are all supported), and the documentation in the comments for
the current Agent precision configuration parameter was changed a bit to reflect the real
value that should be set if you want to measure timestamps in times other than seconds
(choosing a value of "ns" for this parameter results in a precision of 0ns, which is then
treated like a precision of 1s by the code since 1s is the default in the case where a
precision value was not specified).

The changes that are made in this pull request will only effect the output of data in a json
data_format (the timestamps for the graphite and influx data_formats remain as they were after this pull request is merged, with the timestamps returned in the graphite data_format having a fixed precision of 1s and timestamps for the the influx data_format having a fixed precision of 1ns, as they always have), and the outputs that use the Serialize method to output data in a json data_format are the only outputs that have been changed. As was stated earlier, using this new code with an existing telegraf.conf file (where this new configuration parameter is not specified for any of those outputs) will result in the same behavior that is seen currently (where all data in a json data_format is output with timestamps that have been truncated to the nearest second, regardless of the precision with which they were measured).

The following output shows the result of these changes when an output_precision value is not specified in the configuration file for a file output with a json data_format (the equivalent to the current released behavior of the telegraf agent), in which case a default value of one second is used:

$ telegraf --config telegraf-sys-file.conf
2016-12-08T03:11:59Z I! Starting Telegraf (version dev-42-g7a1a91d)
2016-12-08T03:11:59Z I! Loaded outputs: file
2016-12-08T03:11:59Z I! Loaded inputs: inputs.system
2016-12-08T03:11:59Z I! Tags enabled: host=Toms-MacBook-Pro.local
2016-12-08T03:11:59Z I! Agent Config: Interval:10s, Quiet:false, Hostname:"Toms-MacBook-Pro.local", Flush Interval:10s
{"fields":{"load1":1.74,"load15":1.55,"load5":1.61,"n_cpus":8,"n_users":9},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166720}
{"fields":{"uptime":2662650,"uptime_format":"30 days, 19:37"},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166720}
{"fields":{"load1":1.63,"load15":1.54,"load5":1.59,"n_cpus":8,"n_users":9},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166730}
{"fields":{"uptime":2662660,"uptime_format":"30 days, 19:37"},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166730}
^C2016-12-08T03:12:48Z I! Hang on, flushing any cached metrics before shutdown

$ 

And this output shows the result when an output_precision value is specified, with the value chosen set to 1ns (so that the timestamps are output in nanosecond precision):

$ telegraf --config telegraf-sys-file-1ns.conf
2016-12-08T03:12:24Z I! Starting Telegraf (version dev-42-g7a1a91d)
2016-12-08T03:12:24Z I! Loaded outputs: file
2016-12-08T03:12:24Z I! Loaded inputs: inputs.system
2016-12-08T03:12:24Z I! Tags enabled: host=Toms-MacBook-Pro.local
2016-12-08T03:12:24Z I! Agent Config: Interval:10s, Quiet:false, Hostname:"Toms-MacBook-Pro.local", Flush Interval:10s
{"fields":{"load1":1.86,"load15":1.56,"load5":1.64,"n_cpus":8,"n_users":9},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166750050507703}
{"fields":{"uptime":2662680,"uptime_format":"30 days, 19:38"},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166750050587563}
{"fields":{"load1":1.8,"load15":1.56,"load5":1.64,"n_cpus":8,"n_users":9},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166760052514430}
{"fields":{"uptime":2662690,"uptime_format":"30 days, 19:38"},"name":"system","tags":{"host":"Toms-MacBook-Pro.local"},"timestamp":1481166760052566832}
^C2016-12-08T03:12:48Z I! Hang on, flushing any cached metrics before shutdown

$ 

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

@sparrc
Copy link
Contributor

sparrc commented Dec 8, 2016

Thank you for the PR but there are few things wrong with this:

  1. I don't agree with changing ns->1ns, ms->1ms, etc., which would also be a breaking change
  2. This change needs to happen at a much lower level. This needs to happen in the same way the the graphite serializer gets configured, which is in config.go, see https://github.com/influxdata/telegraf/blob/master/internal/config/config.go#L1240-L1278
  3. "precision" isn't the right word here, I would call it "timestamp_unit"

@sparrc sparrc closed this Dec 8, 2016
@tjmcs
Copy link
Author

tjmcs commented Dec 9, 2016

@sparrc; I'm more than happy to continue to try to sort out the best way to approach this; based on your comments (above) I do have a couple of additional questions:

From your comments (above), it looks like you would like to have this new timestamp_unit parameter added as part of the Agent configuration. Is that what you were thinking or would you rather have it added elsewhere in the configuration file? Should it be a part of the metric configuration itself, for example, since that's where it'll be used (when serializing the metric)?

If you want it added as part of the Agent configuration, I'm having a tough time seeing how I can get a reference to that parameter (the timestamp_unit parameter) from within the Serialize method that is bound to the JsonSerializer type without modifying the Serializer interface implemented by that type. If, on the other hand, I add the timestamp_unit parameter as to thetelegraf.Metric type itself then I'll have easy access to it when serializing that object (since the metric is passed in as part of the serialization process).

As far as the change I made from ns to 1ns, ms to 1ms, etc. I realized after submitting the pull request that I probably should have opened a separate issue around that problem. The current codebase takes a string like ns and successfully parses it as a Duration; unfortunately that duration is 0ns, or 0s, and the result is that regardless of the units I choose for the precision (or for this new timestamp_units parameter), I need to use a string like 1ns to get it to evaluate to a non-zero value and return values that have a precision of nanoseconds (if I do what the comments in the code suggest and set the precision to ns, the code actually returns timestamp values to the nearest second, not the nearest nanosecond).

@sparrc
Copy link
Contributor

sparrc commented Dec 9, 2016

sorry but I don't have time to help too much on this. The best advice I can give you is to try to follow the code when the graphite serializer is configured, which extracts the serializer-specific arguments before the configuration gets to the plugins themselves.

as for ns/1ns, this should be a non-issue and easy to deal with, just switch on the value given. There's only a few options so it shouldn't be difficult.

@tjmcs tjmcs deleted the tb/fix-json-timestamp-precision branch March 24, 2017 03:49
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.

JSON output data format, support nanosecond timestamps.
2 participants