search

SumoLogic / fluentd-output-sumologic

Fluentd output plugin to deliver logs or metrics to Sumo Logic.
https://rubygems.org/gems/fluent-plugin-sumologic_output
Apache License 2.0
29 stars 28 forks source link

fix: fix how `compress` configuration flag works #90

Closed andrzej-stencel closed 9 months ago

andrzej-stencel commented 9 months ago

In v1.8.0, setting compress flag to either true or false caused compression to be enabled. This fixes multiple problems in the configure method of the plugin.

sumo-drosiek commented 9 months ago

@astencel-sumo Could you explain what has been changed here? Why previous code didn't work?

andrzej-stencel commented 9 months ago

@astencel-sumo Could you explain what has been changed here? Why previous code didn't work?

Sure.

  1. The conf map in the configure method contains only the values specified by user in configuration, in their raw string form. This means that if user hasn't specified e.g. compress in their config, the value of conf['compress'] is nil. In effect, the SumologicConnection.new method was being called with nils as parameter values for all config properties not explicitly specified by the user. This worked fine for when user did not specify compress when the default was false, but, due to the way the code in the SumologicConnection class is written and the dynamic nature of Ruby, this resulted in compression kicking in when user specified either true or false as the value of compress. This was because the value of the compress_enabled parametr to the SumologicConnection.initialize method was a string (either true or false), and when used in an if condition, it was evaluated as boolean true.
  2. The correct way to write code in the configure method is to use the instance variables like @compress, as described in Fluentd docs. Those variables are correctly initialized to either the value specified by the user (already parsed to the right type), or the default values specified in the plugin. This was not the case in our code, but was fixed by moving the invocation of the super method from the bottom of the configure method to its top (with the exception of the compat_parameters_convert method that goes before the super method).
  3. The other changes in the code and in the unit tests are a result of fixing those issues. For example, the code in the configure method checking the correctness of a conf['metrics_data_type'] was invalid - there's no such configuration option, there's metric_data_format instead. :shrug: