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

Add target option to jdbc input #69

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 5.1.0
- Added `target` option to JDBC input, allowing the row columns to target a specific field instead of being expanded
at the root of the event. This allows the input to play nicer with the Elastic Common Schema when
the input does not follow the schema. [#69](https://github.com/logstash-plugins/logstash-integration-jdbc/issues/69)

## 5.0.7
- Feat: try hard to log Java cause (chain) [#62](https://github.com/logstash-plugins/logstash-integration-jdbc/pull/62)

Expand Down
12 changes: 12 additions & 0 deletions docs/input-jdbc.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ This plugin supports the following configuration options plus the <<plugins-{typ
| <<plugins-{type}s-{plugin}-sql_log_level>> |<<string,string>>, one of `["fatal", "error", "warn", "info", "debug"]`|No
| <<plugins-{type}s-{plugin}-statement>> |<<string,string>>|No
| <<plugins-{type}s-{plugin}-statement_filepath>> |a valid filesystem path|No
| <<plugins-{type}s-{plugin}-target>> | {logstash-ref}/field-references-deepdive.html[field reference] | No
| <<plugins-{type}s-{plugin}-tracking_column>> |<<string,string>>|No
| <<plugins-{type}s-{plugin}-tracking_column_type>> |<<string,string>>, one of `["numeric", "timestamp"]`|No
| <<plugins-{type}s-{plugin}-use_column_value>> |<<boolean,boolean>>|No
Expand Down Expand Up @@ -535,6 +536,17 @@ with the `parameters` setting.

Path of file containing statement to execute

[id="plugins-{type}s-{plugin}-target"]
===== `target`

* Value type is {logstash-ref}/field-references-deepdive.html[field reference]
* There is no default value for this setting.

Without a `target`, events are created from each row column at the root level.
When the `target` is set to a field reference, the column of each row is placed in the target field instead.

This option can be useful to avoid populating unknown fields when a downstream schema such as ECS is enforced.

[id="plugins-{type}s-{plugin}-tracking_column"]
===== `tracking_column`

Expand Down
22 changes: 21 additions & 1 deletion lib/logstash/inputs/jdbc.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require "logstash/namespace"
require "logstash/plugin_mixins/jdbc/common"
require "logstash/plugin_mixins/jdbc/jdbc"
require "logstash/plugin_mixins/ecs_compatibility_support"
require "logstash/plugin_mixins/validator_support/field_reference_validation_adapter"

# this require_relative returns early unless the JRuby version is between 9.2.0.0 and 9.2.8.0
require_relative "tzinfo_jruby_patch"
Expand Down Expand Up @@ -129,6 +131,11 @@
module LogStash module Inputs class Jdbc < LogStash::Inputs::Base
include LogStash::PluginMixins::Jdbc::Common
include LogStash::PluginMixins::Jdbc::Jdbc
# adds ecs_compatibility config which could be :disabled or :v1
include LogStash::PluginMixins::ECSCompatibilitySupport(:disabled,:v1,:v8 => :v1)
# adds :field_reference validator adapter
extend LogStash::PluginMixins::ValidatorSupport::FieldReferenceValidationAdapter

config_name "jdbc"

# If undefined, Logstash will complain, even if codec is unused.
Expand Down Expand Up @@ -209,6 +216,9 @@ module LogStash module Inputs class Jdbc < LogStash::Inputs::Base

config :prepared_statement_bind_values, :validate => :array, :default => []

# Define the target field to store the loaded columns
config :target, :validate => :field_reference, :required => false

attr_reader :database # for test mocking/stubbing

public
Expand Down Expand Up @@ -260,6 +270,11 @@ def register
converters[encoding] = converter
end
end

# target must be populated if ecs_compatibility is not :disabled
if @target.nil? && ecs_compatibility != :disabled
logger.warn("When ECS compatibility is enabled also target option must be valued")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

believe we agreed upon this being logger at the info level (as a compromise since it might be legit to skip setting target in cases where data has no conflicts with the schema).

what I would be also interested, due other plugins, is a standard message we would use across plugins ... draft:
ECS compatibility is enabled but no ``target`` option was specified, it is recommended to set the option to avoid potential schema conflicts (if you're data is ECS compliant or non-conflicting feel free to ignore this message) // cc @yaauie @karenzone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on standardize, maybe we can put it as constant in the ECS mixin?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kares In another conversation for a different plugin, I believe we discussed emitting this as info, since it isn't exactly something that needs to be resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you recall maybe the plugin?So that I can use the same log info, to be consistent across various plugins

Copy link
Contributor

@kares kares May 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

believe we agreed upon this being logger at the info level

yes we agreed on info level logging previously, we do not have another plugin with the "warning" yet.
that's why I proposed a message for review, if accepted we would use it here and maybe move to ECS support mixing for re-use (as @andsel suggested)? (it's going to concern 3 other plugins I can think of right away)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger.warn("When ECS compatibility is enabled also target option must be valued")
logger.info("ECS compatibility is enabled but no ``target`` option was specified, it is recommended to set the option to avoid potential schema conflicts (if you're data is ECS compliant or non-conflicting feel free to ignore this message)")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your message is perfect, in case of jdbc static and filter we would also add the id of the interested lookups, so in that case it could hit many lines like this, one for each lookup definition

end
end # def register

# test injection points
Expand Down Expand Up @@ -318,7 +333,12 @@ def execute_query(queue)
## do the necessary conversions to string elements
row = Hash[row.map { |k, v| [k.to_s, convert(k, v)] }]
end
event = LogStash::Event.new(row)
if @target
event = LogStash::Event.new
event.set(@target, row)
else
event = LogStash::Event.new(row)
end
decorate(event)
queue << event
end
Expand Down
4 changes: 3 additions & 1 deletion logstash-integration-jdbc.gemspec
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
Gem::Specification.new do |s|
s.name = 'logstash-integration-jdbc'
s.version = '5.0.7'
s.version = '5.1.0'
s.licenses = ['Apache License (2.0)']
s.summary = "Integration with JDBC - input and filter plugins"
s.description = "This gem is a Logstash plugin required to be installed on top of the Logstash core pipeline using $LS_HOME/bin/logstash-plugin install gemname. This gem is not a stand-alone program"
Expand Down Expand Up @@ -36,6 +36,8 @@ Gem::Specification.new do |s|
s.add_runtime_dependency 'tzinfo-data'
# 3.5 limitation is required for jdbc-static loading schedule
s.add_runtime_dependency 'rufus-scheduler', '< 3.5'
s.add_runtime_dependency 'logstash-mixin-ecs_compatibility_support', '~>1.2'
s.add_runtime_dependency "logstash-mixin-validator_support", '~> 1.0'

s.add_development_dependency "childprocess"
s.add_development_dependency 'logstash-devutils'
Expand Down
42 changes: 42 additions & 0 deletions spec/inputs/jdbc_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,48 @@

end

context "when using target option" do
let(:settings) do
{
"statement" => "SELECT * from test_table FETCH FIRST 1 ROWS ONLY",
"target" => "sub_field"
}
end

before do
plugin.register
end

after do
plugin.stop
end

it "should put all columns under sub-field" do
db[:test_table].insert(:num => 1, :custom_time => Time.now.utc, :created_at => Time.now.utc, :string => "Test target option")

plugin.run(queue)

expect(queue.size).to eq(1)
event = queue.pop
expect(event.get("[sub_field][string]")).to eq("Test target option")
end
end

context "when using target option is not set and ecs_compatibility is enabled" do
let(:settings) do
{
"statement" => "SELECT * from test_table FETCH FIRST 1 ROWS ONLY",
"ecs_compatibility" => :v1
}
end

it "should log a warn of missed target usage" do
expect(plugin.logger).to receive(:warn).once.with("When ECS compatibility is enabled also target option must be valued")

plugin.register
end
end

context "when fetching time data" do

let(:settings) do
Expand Down