Merge pull request #971 from fluent/show-deprecated-warnings-for-rese…

…rved-params

Show deprecated warnings for reserved params

lib/fluent/agent.rb

@@ -62,7 +62,7 @@ def configure(conf)
       # initialize <match> and <filter> elements
       conf.elements('filter', 'match').each { |e|
         pattern = e.arg.empty? ? '**' : e.arg
-        type = e['@type'] || e['type']
+        type = e['@type']
         if e.name == 'filter'
           add_filter(type, pattern, e)
         else

lib/fluent/compat/output.rb

@@ -35,13 +35,6 @@ def self.buffer_section(conf)
       def self.secondary_section(conf)
         conf.elements(name: 'secondary').first
       end
-
-      def self.inject_type_from_obsoleted_name(secconf, log)
-        if secconf['type'] && !secconf['@type']
-          secconf['@type'] = secconf['type']
-          log.warn "'type' is deprecated, and will be ignored in v1: use '@type' instead."
-        end
-      end
     end
 
     module BufferedEventStreamMixin
@@ -236,14 +229,7 @@ def configure(conf)
             buf_params[newer] = conf[older] if conf.has_key?(older)
           end
 
-          bufconf = Fluent::Config::Element.new('buffer', '', buf_params, [])
-
-          conf.elements << bufconf
-
-          secconf = CompatOutputUtils.secondary_section(conf)
-          if secconf
-            CompatOutputUtils.inject_type_from_obsoleted_name(secconf, log)
-          end
+          conf.elements << Fluent::Config::Element.new('buffer', '', buf_params, [])
         end
 
         methods_of_plugin = self.class.instance_methods(false)
@@ -372,14 +358,7 @@ def configure(conf)
             buf_params[newer] = conf[older] if conf.has_key?(older)
           end
 
-          bufconf = Fluent::Config::Element.new('buffer', 'tag', buf_params, [])
-
-          conf.elements << bufconf
-
-          secconf = CompatOutputUtils.secondary_section(conf)
-          if secconf
-            CompatOutputUtils.inject_type_from_obsoleted_name(secconf, log)
-          end
+          conf.elements << Fluent::Config::Element.new('buffer', 'tag', buf_params, [])
         end
 
         super
@@ -540,14 +519,7 @@ def configure(conf)
                             end
           buf_params["timekey_range"] = @_timekey_range
 
-          bufconf = Fluent::Config::Element.new('buffer', 'time', buf_params, [])
-
-          conf.elements << bufconf
-
-          secconf = CompatOutputUtils.secondary_section(conf)
-          if secconf
-            CompatOutputUtils.inject_type_from_obsoleted_name(secconf, log)
-          end
+          conf.elements << Fluent::Config::Element.new('buffer', 'time', buf_params, [])
         end
 
         super

lib/fluent/config/element.rb

@@ -32,11 +32,22 @@ def initialize(name, arg, attrs, elements, unused = nil)
         @v1_config = false
         @corresponding_proxies = [] # some plugins use flat parameters, e.g. in_http doesn't provide <format> section for parser.
         @unused_in = false # if this element is not used in plugins, correspoing plugin name and parent element name is set, e.g. [source, plugin class].
+
+        # it's global logger, not plugin logger: deprecated message should be global warning, not plugin level.
+        @logger = defined?($log) ? $log : nil
       end
 
       attr_accessor :name, :arg, :unused, :v1_config, :corresponding_proxies, :unused_in
       attr_writer :elements
 
+      RESERVED_PARAMETERS_COMPAT = {
+        '@type' => 'type',
+        '@id' => 'id',
+        '@log_level' => 'log_level',
+        '@label' => nil,
+      }
+      RESERVED_PARAMETERS = RESERVED_PARAMETERS_COMPAT.keys
+
       def elements(*names, name: nil, arg: nil)
         raise ArgumentError, "name and names are exclusive" if name && !names.empty?
         raise ArgumentError, "arg is available only with name" if arg && !name
@@ -99,6 +110,12 @@ def has_key?(key)
       def [](key)
         @unused_in = false # ditto
         @unused.delete(key)
+
+        if RESERVED_PARAMETERS.include?(key) && !has_key?(key) && has_key?(RESERVED_PARAMETERS_COMPAT[key])
+          @logger.warn "'#{RESERVED_PARAMETERS_COMPAT[key]}' is deprecated parameter name. use '#{key}' instead." if @logger
+          return self[RESERVED_PARAMETERS_COMPAT[key]]
+        end
+
         super
       end
 

lib/fluent/config/section.rb

@@ -119,7 +119,7 @@ def self.generate(proxy, conf, logger, plugin_class, stack = [])
             section_params[key] = self.instance_exec(conf.arg, opts, name, &block)
           end
           unless section_params.has_key?(proxy.argument.first)
-            logger.error "config error in:\n#{conf}"
+            logger.error "config error in:\n#{conf}" if logger # logger should exist, but somethimes it's nil (e.g, in tests)
             raise ConfigError, "'<#{proxy.name} ARG>' section requires argument" + section_stack
           end
         end
@@ -136,7 +136,7 @@ def self.generate(proxy, conf, logger, plugin_class, stack = [])
             section_params[varname] = self.instance_exec(val, opts, name, &block)
           end
           unless section_params.has_key?(varname)
-            logger.error "config error in:\n#{conf}"
+            logger.error "config error in:\n#{conf}" if logger
             raise ConfigError, "'#{name}' parameter is required" + section_stack
           end
         end
@@ -156,14 +156,14 @@ def self.generate(proxy, conf, logger, plugin_class, stack = [])
           }
 
           if subproxy.required? && elements.size < 1
-            logger.error "config error in:\n#{conf}"
+            logger.error "config error in:\n#{conf}" if logger
             raise ConfigError, "'<#{subproxy.name}>' sections are required" + section_stack
           end
           if subproxy.multi?
             section_params[varname] = elements.map{ |e| generate(subproxy, e, logger, plugin_class, stack + [subproxy.name]) }
           else
             if elements.size > 1
-              logger.error "config error in:\n#{conf}"
+              logger.error "config error in:\n#{conf}" if logger
               raise ConfigError, "'<#{subproxy.name}>' section cannot be written twice or more" + section_stack
             end
             section_params[varname] = generate(subproxy, elements.first, logger, plugin_class, stack + [subproxy.name])

lib/fluent/configurable.rb

@@ -32,11 +32,13 @@ def initialize
       # to simulate implicit 'attr_accessor' by config_param / config_section and its value by config_set_default
       proxy = self.class.merged_configure_proxy
       proxy.params.keys.each do |name|
+        next if name.to_s.start_with?('@')
         if proxy.defaults.has_key?(name)
           instance_variable_set("@#{name}".to_sym, proxy.defaults[name])
         end
       end
       proxy.sections.keys.each do |name|
+        next if name.to_s.start_with?('@')
         subproxy = proxy.sections[name]
         if subproxy.multi?
           instance_variable_set("@#{subproxy.param_name}".to_sym, [])
@@ -49,7 +51,7 @@ def initialize
     def configure(conf)
       @config = conf
 
-      logger = self.respond_to?(:log) ? log : $log
+      logger = self.respond_to?(:log) ? log : (defined?($log) ? $log : nil)
       proxy = self.class.merged_configure_proxy
       conf.corresponding_proxies << proxy
 
@@ -67,6 +69,7 @@ def configure(conf)
       @config_root_section = root
 
       root.instance_eval{ @params.keys }.each do |param_name|
+        next if param_name.to_s.start_with?('@')
         varname = "@#{param_name}".to_sym
         if (! root[param_name].nil?) || (instance_variable_defined?(varname) && instance_variable_get(varname).nil?)
           instance_variable_set(varname, root[param_name])
@@ -128,7 +131,8 @@ def configured_in(section_name)
 
       def config_param(name, type = nil, **kwargs, &block)
         configure_proxy(self.name).config_param(name, type, **kwargs, &block)
-        attr_accessor name
+        # reserved names '@foo' are invalid as attr_accessor name
+        attr_accessor(name) unless Fluent::Config::Element::RESERVED_PARAMETERS.include?(name.to_s)
       end
 
       def config_set_default(name, defval)

lib/fluent/log.rb

@@ -407,7 +407,7 @@ module PluginLoggerMixin
     def self.included(klass)
       klass.instance_eval {
         desc 'Allows the user to set different levels of logging for each plugin.'
-        config_param :log_level, :string, default: nil, alias: :@log_level
+        config_param :@log_level, :string, default: nil, alias: :log_level # 'log_level' will be warned as deprecated
       }
     end
 
@@ -422,11 +422,11 @@ def initialize
     def configure(conf)
       super
 
-      if @log_level
+      if level = conf['@log_level']
         unless @log.is_a?(PluginLogger)
           @log = PluginLogger.new($log.dup)
         end
-        @log.level = @log_level
+        @log.level = level
         @log.optional_header = "[#{self.class.name}#{plugin_id_configured? ? "(" + @id + ")" : ""}] "
         @log.optional_attrs = {}
       end

lib/fluent/plugin/in_monitor_agent.rb

@@ -368,7 +368,7 @@ def plugin_info_by_id(plugin_id, opts={})
     # multiple plugins could have the same type
     def plugins_info_by_type(type, opts={})
       array = all_plugins.select {|pe|
-        (pe.config['@type'] == type || pe.config['type'] == type) rescue nil
+        (pe.config['@type'] == type) rescue nil
       }
       array.map {|pe|
         get_monitor_info(pe, opts)
@@ -391,7 +391,7 @@ def get_monitor_info(pe, opts={})
       # Common plugin information
       obj['plugin_id'] = pe.plugin_id
       obj['plugin_category'] = plugin_category(pe)
-      obj['type'] = pe.config['@type'] || pe.config['type']
+      obj['type'] = pe.config['@type']
       obj['config'] = pe.config if !opts.has_key?(:with_config) || opts[:with_config]
 
       # run MONITOR_INFO in plugins' instance context and store the info to obj

lib/fluent/plugin/multi_output.rb

@@ -57,11 +57,7 @@ def configure(conf)
 
         @stores.each do |store|
           store_conf = store.corresponding_config_element
-          type = store[:@type]
-          if !type && store_conf['type']
-            type = store_conf['type']
-            log.warn "'type' is deprecated, and will be ignored in v1: use '@type' instead."
-          end
+          type = store_conf['@type']
           unless type
             raise Fluent::ConfigError, "Missing '@type' parameter in <store> section"
           end

lib/fluent/plugin/out_copy.rb

@@ -37,7 +37,7 @@ def configure(conf)
       conf.elements.select {|e|
         e.name == 'store'
       }.each {|e|
-        type = e['@type'] || e['type']
+        type = e['@type']
         unless type
           raise ConfigError, "Missing 'type' parameter on <store> directive"
         end

lib/fluent/plugin/out_roundrobin.rb

@@ -37,7 +37,7 @@ def configure(conf)
       conf.elements.select {|e|
         e.name == 'store'
       }.each {|e|
-        type = e['@type'] || e['type']
+        type = e['@type']
         unless type
           raise ConfigError, "Missing 'type' parameter on <store> directive"
         end

lib/fluent/plugin/output.rb

@@ -42,7 +42,7 @@ class Output < Base
       # `<buffer>` and `<secondary>` sections are available only when '#format' and '#write' are implemented
       config_section :buffer, param_name: :buffer_config, init: true, required: false, multi: false, final: true do
         config_argument :chunk_keys, :array, value_type: :string, default: []
-        config_param :@type, :string, default: 'memory'
+        config_param :@type, :string, default: 'memory', alias: :type
 
         config_param :timekey_range, :time, default: nil # range size to be used: `time.to_i / @timekey_range`
         config_param :timekey_wait, :time, default: 600
@@ -90,7 +90,7 @@ class Output < Base
       end
 
       config_section :secondary, param_name: :secondary_config, required: false, multi: false, final: true do
-        config_param :@type, :string, default: nil
+        config_param :@type, :string, default: nil, alias: :type
         config_section :buffer, required: false, multi: false do
           # dummy to detect invalid specification for here
         end

lib/fluent/plugin_id.rb

@@ -21,7 +21,7 @@ module PluginId
     @@configured_ids = Set.new
 
     def configure(conf)
-      @id = conf['@id'] || conf['id']
+      @id = conf['@id']
       @_id_configured = !!@id # plugin id is explicitly configured by users (or not)
       if @id
         @id = @id.to_s

lib/fluent/root_agent.rb

@@ -90,7 +90,7 @@ def configure(conf)
         log.info "'--without-source' is applied. Ignore <source> sections"
       else
         conf.elements(name: 'source').each { |e|
-          type = e['@type'] || e['type']
+          type = e['@type']
           raise ConfigError, "Missing 'type' parameter on <source> directive" unless type
           add_source(type, e)
         }

test/plugin/test_multi_output.rb

@@ -147,7 +147,7 @@ def create_output(type=:multi)
       assert_equal 4, @i.outputs.size
 
       logs = @i.log.out.logs
-      assert{ logs.select{|log| log.include?('[warn]') && log.include?("'type' is deprecated, and will be ignored in v1: use '@type' instead.") }.size == 4 }
+      assert{ logs.select{|log| log.include?('[warn]') && log.include?("'type' is deprecated parameter name. use '@type' instead.") }.size == 4 }
     end
 
     test '#emit_events calls #process always' do