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

Ensure that support for custom config setting validation is included in Java API design. #9733

Open
guyboertje opened this issue Jun 12, 2018 · 0 comments

Comments

@guyboertje
Copy link
Contributor

guyboertje commented Jun 12, 2018

The intention is to ensure that custom validation strategies are not forgotten when the Java Plugin API is finalised and built.

Prior art:
#5778
#7986

Current Java API for plugin META issue - #9215

Background:

  • There are some plugins that do extra validation steps in the register method, the jdbc input and kafka output are examples.
  • There are some plugins that do extra validation stepsby defining an initialize method where validation is done after calling super,
  • Validation in the initialize or register methods does not participate in the config validation stage of the pipeline loading.

The jdbc_static filter does have complex validation that participates in the config validation stage as does the file input. There are plenty on plugins that could benefit from a similar mechanism.

An invalid setting in the jdbc_static config is displayed well with clear instructions and multiple invalid config options are shown where possible:

[2018-06-12T11:29:50,877][INFO ][logstash.modules.scaffold] Initializing module {:module_name=>"netflow", :directory=>"/elastic/tmp/logstash-5.6.10/modules/netflow/configuration"}
[2018-06-12T11:29:50,884][INFO ][logstash.modules.scaffold] Initializing module {:module_name=>"fb_apache", :directory=>"/elastic/tmp/logstash-5.6.10/modules/fb_apache/configuration"}
[2018-06-12T11:29:51,403][ERROR][logstash.filters.jdbcstatic] Invalid setting for jdbc_static filter plugin:

  filter {
    jdbc_static {
      # This setting must be a [LogStash::Filters::Jdbc::Loader]
      # The options must include a 'local_table' string, The options for 'server' must include a 'query' string
      loaders => [{"id"=>"servers"}]
      ...
    }
  }
[2018-06-12T11:29:51,413][ERROR][logstash.agent           ] Cannot create pipeline {:reason=>"Something is wrong with your configuration."}

The standard validation output for a invalid setting that is similarly well displayed:

[2018-06-12T12:16:07,657][INFO ][logstash.modules.scaffold] Initializing module {:module_name=>"netflow", :directory=>"/elastic/tmp/logstash-5.6.10/modules/netflow/configuration"}
[2018-06-12T12:16:07,662][INFO ][logstash.modules.scaffold] Initializing module {:module_name=>"fb_apache", :directory=>"/elastic/tmp/logstash-5.6.10/modules/fb_apache/configuration"}
[2018-06-12T12:16:07,829][ERROR][logstash.filters.geoip   ] Missing a required setting for the geoip filter plugin:

  filter {
    geoip {
      source => # SETTING MISSING
      ...
    }
  }
[2018-06-12T12:16:07,836][ERROR][logstash.agent           ] Cannot create pipeline {:reason=>"Something is wrong with your configuration."}

Whereas an invalid setting in the jdbc input is very ugly and unhelpful with (usually) only the first invalid error being shown.

[2018-06-12T11:41:21,627][INFO ][logstash.modules.scaffold] Initializing module {:module_name=>"netflow", :directory=>"/elastic/tmp/logstash-5.6.10/modules/netflow/configuration"}
[2018-06-12T11:41:21,632][INFO ][logstash.modules.scaffold] Initializing module {:module_name=>"fb_apache", :directory=>"/elastic/tmp/logstash-5.6.10/modules/fb_apache/configuration"}
[2018-06-12T11:41:21,855][INFO ][logstash.pipeline        ] Starting pipeline {"id"=>"main", "pipeline.workers"=>8, "pipeline.batch.size"=>125, "pipeline.batch.delay"=>5, "pipeline.max_inflight"=>1000}
[2018-06-12T11:41:22,038][ERROR][logstash.pipeline        ] Error registering plugin {:plugin=>"<LogStash::Inputs::Jdbc use_column_value=>true, jdbc_driver_library=>\"mysql-connector-java-5.1.36-bin.jar\", jdbc_driver_class=>\"com.mysql.jdbc.Driver\", jdbc_connection_string=>\"jdbc:mysql://localhost:3306/mydb\", jdbc_user=>\"mysql\", parameters=>{\"favorite_artist\"=>\"Beethoven\"}, schedule=>\"* * * * *\", statement=>\"SELECT * from songs where artist = :favorite_artist\", id=>\"116712388977aa612e99784a9f7434fb16f45092-1\", enable_metric=>true, codec=><LogStash::Codecs::Plain id=>\"plain_26b64880-f095-4c29-a551-d213c610739e\", enable_metric=>true, charset=>\"UTF-8\">, jdbc_paging_enabled=>false, jdbc_page_size=>100000, jdbc_validate_connection=>false, jdbc_validation_timeout=>3600, jdbc_pool_timeout=>5, sql_log_level=>\"info\", connection_retry_attempts=>1, connection_retry_attempts_wait_time=>0.5, last_run_metadata_path=>\"/Users/guy/.logstash_jdbc_last_run\", tracking_column_type=>\"numeric\", clean_run=>false, record_last_run=>true, lowercase_column_names=>true>", :error=>"Must set :tracking_column if :use_column_value is true."}
[2018-06-12T11:41:22,123][ERROR][logstash.agent           ] Pipeline aborted due to error {:exception=>#<LogStash::ConfigurationError: Must set :tracking_column if :use_column_value is true.>, :backtrace=>["/elastic/tmp/logstash-5.6.10/vendor/bundle/jruby/1.9/gems/logstash-input-jdbc-4.3.9/lib/logstash/inputs/jdbc.rb:212:in `register'", "/elastic/tmp/logstash-5.6.10/logstash-core/lib/logstash/pipeline.rb:290:in `register_plugin'", "/elastic/tmp/logstash-5.6.10/logstash-core/lib/logstash/pipeline.rb:301:in `register_plugins'", "org/jruby/RubyArray.java:1613:in `each'", "/elastic/tmp/logstash-5.6.10/logstash-core/lib/logstash/pipeline.rb:301:in `register_plugins'", "/elastic/tmp/logstash-5.6.10/logstash-core/lib/logstash/pipeline.rb:456:in `start_inputs'", "/elastic/tmp/logstash-5.6.10/logstash-core/lib/logstash/pipeline.rb:348:in `start_workers'", "/elastic/tmp/logstash-5.6.10/logstash-core/lib/logstash/pipeline.rb:235:in `run'", "/elastic/tmp/logstash-5.6.10/logstash-core/lib/logstash/agent.rb:408:in `start_pipeline'"]}
[2018-06-12T11:41:22,173][INFO ][logstash.agent           ] Successfully started Logstash API endpoint {:port=>9600}
[2018-06-12T11:41:25,145][WARN ][logstash.agent           ] stopping pipeline {:id=>"main"}

The above is an example of co-dependent settings requiring additional validation.

Also, register or initialize based validation errors are never seen when using the -t command line switch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant