search

logstash-plugins / logstash-output-influxdb

Apache License 2.0
58 stars 79 forks source link

Change default retention policy from "default" to "autogen" #53

Open cslysy opened 7 years ago

cslysy commented 7 years ago

New version of influxdb use "autogen" as default retention policy name - please see https://github.com/influxdata/influxdb/issues/3733 for more info.

Plugin configuration need to be adjusted from:

# The retention policy to use
  config :retention_policy, :validate => :string, :default => "default"

to

# The retention policy to use
  config :retention_policy, :validate => :string, :default => "autogen"
notfol commented 7 years ago

+1

PauloAugusto-Asos commented 7 years ago

I think I would prefer to have the "default" Retention Policy Name be something that indicates that it should be changed. Ex: "Change-Me".

That way while forgetting so change it, one won't automatically send it to the wrong RP. (as a rule, we should create a new RP and use that one instead)

But I'm a bit divided by that. If that's the case, it should no longer be an "optional" parameter. Not being an optional parameter it makes it harder to accidentally send the data to the "wrong" RP but it also makes it a wee bit harder to make it work out of the box.

As far as I'm concerned, I prefer for this parameter to no longer be optional and have the config template propose the value of "autogen" so that you can just uncomment it.

Ultimation commented 7 years ago

This looks done in https://github.com/logstash-plugins/logstash-output-influxdb/commit/8d2324af9b3127fc725800d57370543054f0de26 ?

sslupsky commented 6 years ago

I am going to chime in with a discussion about what is meant by the "default" retention policy. Influx defines the "default" retention policy as the policy that is specified as the default when you create the retention policy. That is, a database can have multiple retention policies and one of those retention policies can be set as the "default".

The Influx output plugin defines the "default" retention policy as something that is not specified. This is not consistent with the influx "default" so I propose this plugin be changed to be consistent with the Influx definition. That is, if no retention policy is specified in the logstash configuration then the data is sent to whatever the "default" is set by influx.

This is opposed to the current situation where the retention policy is not specified, then the retention policy is forced to "autogen" and potentially overrides the default setting in Influx.

Philosophically speaking, I want to decision about which retention policy the data goes to to be determined by Influx, not logstash, unless explicitly specified in logstash.

So, I propose the following:

config :retention_policy, :validate => :string, :default => "autogen"

be changed to:

config :retention_policy, :validate => :string, :default => nil

sslupsky commented 6 years ago

This would be a breaking change for anyone where the retention policy was not specified in logstash and the default setting set by Influx is not "autogen".

If we do not want a breaking change, then I propose we create another parameter boolean "use_default_retention_policy". If true, then the retention_policy is set to nil so that Influx will use the default. If false, the retention_policy is set to what ever is specified by "retention_policy".