Adds a new json_timestamp_units configuration parameter #2587
Conversation
internal/config/config.go
Outdated
| if c.DataFormat == "json" { | ||
| c.TimestampUnits = agentConfig.JsonTimestampUnits | ||
| } | ||
|
|
There was a problem hiding this comment.
You should parse the json_timestamp_units option here, and convert it to a time.Duration. It should look similar to the parsing of "prefix" and "template". All of the other changes in this file will then be unneeded.
There was a problem hiding this comment.
The json_timestamp_units (as I defined it in this PR) is actually part of the agent configuration, not part of the configuration for the output filters (this was in response to my previous PRs on this topic, where the feedback I got was that I shouldn't have to touch any of the outputs; all of the changes should be made in the serializer itself). Since it's not part of the output filter, it's not available in the pluginSubTable that is passed into this method from the LoadConfig method (through the addOutput method). Is there a way to get to the AgentConfig from within this method? If so, I'd be more than happy to make the change, but as things stand if I modify this to look more like the code that parses the prefix and template values for the graphite output filter, the end result is that the TimestampUnits are never set for the serializer configuration (since the json_timestamp_units are not part of the pluginSubTable being parsed here). Does that make sense?
plugins/serializers/json/json.go
Outdated
| } | ||
|
|
||
| func (s *JsonSerializer) Serialize(metric telegraf.Metric) ([]byte, error) { | ||
| m := make(map[string]interface{}) | ||
| m["tags"] = metric.Tags() | ||
| m["fields"] = metric.Fields() | ||
| m["name"] = metric.Name() | ||
| m["timestamp"] = metric.UnixNano() / 1000000000 | ||
| m["timestamp"] = metric.UnixNano() / s.TimestampUnits.Duration.Nanoseconds() |
There was a problem hiding this comment.
Guard against the duration being 0s or negative, just emit them at the default units.
| if c.DataFormat == "json" { | ||
| c.TimestampUnits = agentConfig.JsonTimestampUnits | ||
| } | ||
|
|
||
| delete(tbl.Fields, "data_format") | ||
| delete(tbl.Fields, "prefix") | ||
| delete(tbl.Fields, "template") |
There was a problem hiding this comment.
Don't forget to delete the field here after making the above change.
internal/config/config.go
Outdated
| @@ -1243,7 +1254,7 @@ func buildParser(name string, tbl *ast.Table) (parsers.Parser, error) { | |||
| // buildSerializer grabs the necessary entries from the ast.Table for creating | |||
| // a serializers.Serializer object, and creates it, which can then be added onto | |||
| // an Output object. | |||
| func buildSerializer(name string, tbl *ast.Table) (serializers.Serializer, error) { | |||
| func buildSerializer(name string, tbl *ast.Table, agentConfig *AgentConfig) (serializers.Serializer, error) { | |||
| c := &serializers.Config{} | |||
There was a problem hiding this comment.
This is where you can set the default of 1s
|
I think I addressed some of your concerns, @danielnelson, but I still have a question regarding how best to add this new feature to the configuration. From the feedback that you provided that I haven't addressed yet (above), it seems that you are suggesting that I add this new From your feedback (above), I can see how adding this to the output filter configurations might not be that invasive as I first thought because I could just parse value from the I've pushed one set of changes in (to make sure that the value defined is not less than or equal to zero and to convert the value passed in from the configuration file into a |
|
I think that finishes off the changes you requested, @danielnelson; with those changes in place the $ cat telegraf-new.conf
# Telegraf Configuration
#
# Telegraf is entirely plugin driven. All metrics are gathered from the
# declared inputs, and sent to the declared outputs.
#
# Plugins must be declared in here to be active.
# To deactivate a plugin, comment out the name and any variables.
#
# Use 'telegraf -config telegraf.conf -test' to see what metrics a config
# file would generate.
#
# Environment variables can be used anywhere in this config file, simply prepend
# them with $. For strings the variable must be within quotes (ie, "$STR_VAR"),
# for numbers and booleans they should be plain (ie, $INT_VAR, $BOOL_VAR)
# Global tags can be specified here in key="value" format.
[global_tags]
# dc = "us-east-1" # will tag all metrics with dc=us-east-1
# rack = "1a"
## Environment variables can be used as tags, and throughout the config file
# user = "$USER"
# Configuration for telegraf agent
[agent]
## Default data collection interval for all inputs
interval = "10s"
## Rounds collection interval to 'interval'
## ie, if interval="10s" then always collect on :00, :10, :20, etc.
round_interval = true
## Telegraf will send metrics to outputs in batches of at most
## metric_batch_size metrics.
## This controls the size of writes that Telegraf sends to output plugins.
metric_batch_size = 1000
## For failed writes, telegraf will cache metric_buffer_limit metrics for each
## output, and will flush this buffer on a successful write. Oldest metrics
## are dropped first when this buffer fills.
## This buffer only fills when writes fail to output plugin(s).
metric_buffer_limit = 10000
## Collection jitter is used to jitter the collection by a random amount.
## Each plugin will sleep for a random time within jitter before collecting.
## This can be used to avoid many plugins querying things like sysfs at the
## same time, which can have a measurable effect on the system.
collection_jitter = "0s"
## Default flushing interval for all outputs. You shouldn't set this below
## interval. Maximum flush_interval will be flush_interval + flush_jitter
flush_interval = "10s"
## Jitter the flush interval by a random amount. This is primarily to avoid
## large write spikes for users running a large number of telegraf instances.
## ie, a jitter of 5s and interval 10s means flushes will happen every 10-15s
flush_jitter = "0s"
## By default, precision will be set to the same timestamp order as the
## collection interval, with the maximum being 1s.
## Precision will NOT be used for service inputs, such as logparser and statsd.
## Valid values are "ns", "us" (or "µs"), "ms", "s".
precision = "1ns"
## Logging configuration:
## Run telegraf with debug log messages.
debug = false
## Run telegraf in quiet mode (error log messages only).
quiet = false
## Specify the log file name. The empty string means to log to stderr.
logfile = ""
## Override default hostname, if empty use os.Hostname()
hostname = ""
## If set to true, do no set the "host" tag in the telegraf agent.
omit_hostname = false
###############################################################################
# OUTPUT PLUGINS #
###############################################################################
# Send telegraf metrics to file(s)
[[outputs.file]]
## Files to write to, "stdout" is a specially handled file.
files = ["stdout", "/tmp/metrics.out"]
## Data format to output.
## Each data format has it's own unique set of configuration options, read
## more about them here:
## https://github.com/influxdata/telegraf/blob/master/docs/DATA_FORMATS_OUTPUT.md
data_format = "json"
json_timestamp_units = "1ns"
###############################################################################
# PROCESSOR PLUGINS #
###############################################################################
# # Print all metrics that pass through this filter.
# [[processors.printer]]
###############################################################################
# AGGREGATOR PLUGINS #
###############################################################################
# # Keep the aggregate min/max of each metric passing through.
# [[aggregators.minmax]]
# ## General Aggregator Arguments:
# ## The period on which to flush & clear the aggregator.
# period = "30s"
# ## If true, the original metric will be dropped by the
# ## aggregator and will not get sent to the output plugins.
# drop_original = false
###############################################################################
# INPUT PLUGINS #
###############################################################################
# Read metrics about system load & uptime
[[inputs.system]]
# no configuration
and the output is the same as was shown previously (outputting nanosecond timestamps for JSON formatted metrics): $ ./telegraf --config telegraf-new.conf
2017-03-29T03:17:20Z I! Starting Telegraf (version dev-103-ge2a5aa1)
2017-03-29T03:17:20Z I! Loaded outputs: file
2017-03-29T03:17:20Z I! Loaded inputs: inputs.system
2017-03-29T03:17:20Z I! Tags enabled: host=localhost.localdomain
2017-03-29T03:17:20Z I! Agent Config: Interval:10s, Quiet:false, Hostname:"localhost.localdomain", Flush Interval:10s
{"fields":{"load1":0,"load15":0.05,"load5":0.01,"n_cpus":1,"n_users":2},"name":"system","tags":{"host":"localhost.localdomain"},"timestamp":1490757450005440657}
{"fields":{"uptime":83023,"uptime_format":"23:03"},"name":"system","tags":{"host":"localhost.localdomain"},"timestamp":1490757450005472372}
{"fields":{"load1":0,"load15":0.05,"load5":0.01,"n_cpus":1,"n_users":2},"name":"system","tags":{"host":"localhost.localdomain"},"timestamp":1490757460008199476}
{"fields":{"uptime":83033,"uptime_format":"23:03"},"name":"system","tags":{"host":"localhost.localdomain"},"timestamp":1490757460008255513} |
|
The most recent commit updates the documentation in the |
internal/config/config.go
Outdated
| @@ -1245,6 +1245,8 @@ func buildParser(name string, tbl *ast.Table) (parsers.Parser, error) { | |||
| // an Output object. | |||
| func buildSerializer(name string, tbl *ast.Table) (serializers.Serializer, error) { | |||
| c := &serializers.Config{} | |||
| // set a default duration (in case one was not defined in this output) | |||
| c.TimestampUnits = time.Duration(1 * time.Second) | |||
There was a problem hiding this comment.
Set this in the initializer for Config
internal/config/config.go
Outdated
| if node, ok := tbl.Fields["json_timestamp_units"]; ok { | ||
| if kv, ok := node.(*ast.KeyValue); ok { | ||
| if str, ok := kv.Value.(*ast.String); ok { | ||
| c.TimestampUnits, _ = time.ParseDuration(str.Value) |
There was a problem hiding this comment.
If an error occurs, add context and return, use fmt.Errorf
docs/DATA_FORMATS_OUTPUT.md
Outdated
| Note that if a `json_timestamp_units` value is not defined, or if the duration | ||
| defined by that timestamp is less than or equal to zero, then the timestamps | ||
| output in the JSON data format serialized Telegraf metrics will be in seconds | ||
| (the default units for these timestamps). |
There was a problem hiding this comment.
These docs are just too verbose. Combine the examples, add a short description as a TOML comment, and try to reduce additional documentation.
|
Thanks for the quick reply, @danielnelson. I just pushed a set of changes based on your last feedback. Let me know if that looks better :) Again, if you want me to rebase and squash this down to a single commit before you merge, I'm more than happy to do so, but I wanted to maintain the commit history while we're iterating over the change. Just let me know at the end if you want this as a single commit or want to see the complete history. |
|
There is a minor compile error that needs fixed. Don't worry about rebase/squash so long as there are no conflicts, I usually use the "Squash and merge" which will apply it all as one commit. |
|
I've built the application locally under OS X (via the $ go version
go version go1.8 darwin/amd64Can you enlighten me as to what the minor compile error is, @danielnelson? |
|
ah...I see the issue @danielnelson...it's a |
|
One of the current test failures will go away if you rebase (the failure in inputs/tail). The other test failure I'm looking into (inputs/socket_listener). |
tjmcs
force-pushed
the
tb/add-json-timestamp-units
branch
from
eb41898 to
ade1335
Compare
|
rebased to the latest commit on master, hope that helps with the tests that are failing |
…he agent section of hte configto the output filter sections
…ew json_timestamp_units field
…cated units to closest power of 10
tjmcs
force-pushed
the
tb/add-json-timestamp-units
branch
from
ade1335 to
5899b9d
Compare
|
rebased again to pickup the change that @phemmer just made...one more round of tests? |
|
I think we are good, tests passed here: https://circleci.com/gh/influxdata/telegraf/6434 |
Follows this change for the JSON serializer: influxdata#2587
Follows this change for the JSON serializer: influxdata#2587
Follows this change for the JSON serializer: influxdata#2587
Resolves #2124
The changes in this pull request add a new
json_timestamp_unitsparameter to the Telegraf agent configuration, giving users the ability to define the which units should be used when reporting back the timestamps associated with a given metric when that metric is output as JSON. Currently, the Telegraf agent simply returns a timestamp for those metrics in seconds when the metrics are output in JSON format (regardless of the precision that was used in generating the timestamp itself), while the same timestamps are returned in nanoseconds for if those metrics are output in the InfluxDB format. Specifically, this pull request changes the Telegraf configuration file and the JSON serializer as follows in order to enable reporting of the timestamps associated with JSON formatted metrics in units other than seconds:JsonTimestampUnitsfield has been added to theAgentConfigstruct that is defined in theinternal/config/config.gofile, and that field is initialized to a default value of one second when a new instance of this struct is created (ensuring that when the changes in this pull request are merged into the master branch the new version of the Telegraf agent will still work with thetelegraf.conffiles that were generated by previous versions of the Telegraf agent).json_timestamp_unitsfield has been added to the[agent]section of the header that is defined in that sameinternal/config/config.gofile; this is the header that is used to fill in the[agent]section of a new Telegraf configuration file when generating that file using atelegraf ... configcommand. Note that we have left this parameter commented out in the resulting configuration file to prove that if it is undefined the timestamps output for JSON formatted metrics will still be in seconds (the default), but the line is there as an example to the user of how they might set this parameter to a value other than the1sdefault that is shown.buildSerializerfunction signature in that same file has been changed to take an additional argument (a pointer to theAgentConfigstruct), and that struct is used within thebuildSerializerfunction to set the newTimestampUnitsfield that we've added to theConfigstruct defined in theplugins/serializers/registry.gofile; this is the same location where thePrefixandTemplatefields used by the Graphite serializer are defined, so we felt that theTimestampUnitsused in outputting JSON formatted metrics should be added to this struct as well.TimestampUnitsfield in theJsonSerializerstruct defined in theplugins/serializers/json/json.gofile, and that new field is used to ensure that the timestamps output by this serializer are in the correct units (by dividing by the number of nanoseconds in the definedTimestampUnitsduration rather than simply dividing the nanoseconds in the metric timestamp by1,000,000,000).NewJsonSerializerfunction defined in theplugins/serializers/registry.gofile has been modified to accept an argument defining the timestampUnits that the serializer should useThis set of changes was the minimal set of changes that we felt we could make that would add the desired behavior while still maintaining the default behavior of the current system (where timestamps for JSON formatted metrics are output in seconds, by default). We were also careful to ensure that the current Telegraf configuration files could still be used with the new version of the Telegraf agent.
To provide an example of how the changes in this pull request work, we first generated a configuration file that outputs the
systemmetrics to thefileoutput filter using the current released version of Telegraf (v1.2.1):We then modified the
precisionin the[agent]section of the configuration file so that the precision is in nanoseconds (i.e. we set theprecisionvalue to1ns), and started up the telegraf agent using the (slightly) modified configuration file, capturing the first set of metrics reported (note that the configuration file generated, above, has thedata_formatfor thefileoutput filter set toinflux, so the timestamps shown in this snippet are in nanoseconds):Next, we modified the
data_formatin ourfileoutput filter so that the metrics would be output in ajsondata format and restarted the agent, the result was that the timestamps that were output by the samefileoutput filter are now in seconds, rather than nanoseconds:So that demonstrates the issue we are resolving in this pull request. Since there is no way to set the units associated with the timestamp for JSON formatted metrics, we cannot get the timestamps for our metrics in any units other than seconds, regardless of the precision with which that timestamp was measured when the metric was collected.
With the changes in this PR, we can generate a new configuration file that looks like this:
Note that this version of the Telegraf agent includes the new parameter (and comments associated with it) in the
[agent]section of the configuration file:Once again, we changed the precision to
1nsin the agent configuration and we set thedata_formatfor ourfileoutput filter tojson; with these changes in place we got the same behavior that we've seen all along in the JSON formatted metrics that are reported by the new version of the Telegraf agent:if, however, we uncomment the
json_timestamp_unitsline in our new configuration file an set it to1nsas well, we will see these same timestamps output in nanoseconds instead of seconds:Required for all PRs: