Support other rufus scheduling options

[ash-darin]

Allow jdbc input to be scheduled with different options supported by rufus scheduler.

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

[cla-checker-service]

💚 CLA has been signed

[robbavey]

@ash-darin Thank you for your contribution! Would you mind signing the CLA?

[ash-darin]

@robbavey I'm currently clearing this with higher up.

I have to critizise that I have to provide an e-mail address before I am even allowed to read it, or is there a hidden link to the PDF somewhere?

[jsvd]

@ash-darin I'll provide that feedback to the team, but just to clarify, the initial name+email form serves only to pre-fill data in the agreement docusign, after introducing the data you get full access to the agreement and the option to start the signing process itself.

[edit] on that next page you can view and download the agreement as a pdf

[ash-darin]

@jsvd on that "next page" I have to agree "to use electronic records and signatures", marked with an "*" as mandatory, or if I want to "Finish later" I have to provide an e-mail address. Otherwise I do not get access to the PDF.

[jsvd]

@ash-darin right, those are docusign's own formalities on agreeing to use digital signing. if you are/your company is not comfortable agreeing to that before seeing the CLA itself I'll reach out internally to find a way to get the pdf to you first.

[ash-darin]

Is there anything else I ought to do / observe?

[jsvd]

Given .every can be implemented with a cron job (e.g. "every 3h" == 0 */3 * * *), could we maybe add only interval which is different: trigger immediately and when it's done wait "interval" before doing it again?

[ash-darin]

Given .every can be implemented with a cron job (e.g. "every 3h" == 0 */3 * * *), could we maybe add only interval which is different: trigger immediately and when it's done wait "interval" before doing it again?

.every ? Maybe you mistyped and meant period?

I don't see the reason to limit us in that way, there is a use case for which period exists next to cron and interval.

period is slightly different from cron, in the sense that it runs from the moment the timer is started (logstash is started), while a cron always starts at 0 (seconds) . This difference can be advantageous if you have multiple, identically configured logstashes (as many people are prone to do with central configuration management / version management). With a cron job (and no special voodoo to randomize values) they would all trigger simultaneously (e.g. a sql query), while jobs configured with `period´ would trigger slight apart (as the logstashes would start slightly apart), though with the same repeating pattern. I had two logstashes where two cronjobs fired so precisely simultaneous that their writing to elasticsearch collided with a "version mismatch".

I would therefore ask that both, period and interval are implemented.

[jsvd]

Thanks for the clarification, could I ask you to add docs for period and interval and their equivalent section to docs/input-jdbc.asciidoc?

Example:

• schedule entry in setting table https://github.com/logstash-plugins/logstash-integration-jdbc/blob/main/docs/input-jdbc.asciidoc?plain=1#L236
• schedule section below https://github.com/logstash-plugins/logstash-integration-jdbc/blob/main/docs/input-jdbc.asciidoc?plain=1#L565-L576

[ash-darin]

Thanks for the clarification, could I ask you to add docs for period and interval and their equivalent section to docs/input-jdbc.asciidoc?

Example:

• schedule entry in setting table https://github.com/logstash-plugins/logstash-integration-jdbc/blob/main/docs/input-jdbc.asciidoc?plain=1#L236
• schedule section below https://github.com/logstash-plugins/logstash-integration-jdbc/blob/main/docs/input-jdbc.asciidoc?plain=1#L565-L576

Of course, should I do this in this PR? Or better not add to this and open a separate one?

[jsvd]

I'd prefer in the same PR so the same changeset introduces the feature and its associated documentation. 🙏

[jsvd]

@ash-darin thanks for the doc work! I created a PR against yours to make the exclusive nature of these settings explicit to the user ash-darin#1

[ash-darin]

@jsvd Hi, I merged your PR but had to remove the else path as it made the build fail for reasons I could not understand. It was "just" the "ok, do nothing" path, so I think that ought to be sufficient?

See here for an example:

https://app.travis-ci.com/github/logstash-plugins/logstash-integration-jdbc/jobs/633122890

The Travis builds still fail in part but not due to anything in my PR it seems to me.

[jsvd]

This is enough to fix it, while keeping the test:

diff --git a/spec/inputs/jdbc_spec.rb b/spec/inputs/jdbc_spec.rb
index 6085c41..4d08ccd 100755
--- a/spec/inputs/jdbc_spec.rb
+++ b/spec/inputs/jdbc_spec.rb
@@ -229,19 +229,28 @@ describe LogStash::Inputs::Jdbc do
   end
 
   describe "scheduling options" do
+    let(:settings) { super().merge("statement" => "SELECT :num_param as num_param FROM SYSIBM.SYSDUMMY1") }
     scheduling_options = ["interval", "schedule", "period"]
     scheduling_options.combination(2).each do |option1, option2|
       context "when using '#{option1}' and '#{option2}' at the same time" do
         let(:settings) { super().merge(option1 => 'a', option2 => 'b') }
         it "raises a configuration error" do
-          expect { plugin.register }.to raise_error(LogStash::ConfigurationError)
+          expect { plugin.register }.to raise_error(LogStash::ConfigurationError, /Use only one/)
         end
       end
     end
     context "when using 'schedule', 'period' and 'interval' at the same time" do
       let(:settings) { super().merge("interval" => "a", "period" => "b", "schedule" => "c") }
       it "raises a configuration error" do
-        expect { plugin.register }.to raise_error(LogStash::ConfigurationError)
+        expect { plugin.register }.to raise_error(LogStash::ConfigurationError, /Use only one/)
+      end
+    end
+    scheduling_options.each do |option|
+      context "when using only '#{option}'" do
+        let(:settings) { super().merge(option => "a") }
+        it "does not raise a configuration error" do
+          expect { plugin.register }.to_not raise_error
+        end
       end
     end
   end

[jsvd]

CI failures are unrelated and due to an issue in the snapshot logstash docker images that will be fixed by tomorrow. LGTM. I'll merge and publish the PR. thanks for the contribution!