-
Notifications
You must be signed in to change notification settings - Fork 207
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: GrokLogstashPluginAttributesMapper #581
ADD: GrokLogstashPluginAttributesMapper #581
Conversation
Signed-off-by: qchea <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #581 +/- ##
=========================================
Coverage 92.29% 92.29%
Complexity 569 569
=========================================
Files 71 71
Lines 1727 1727
Branches 144 144
=========================================
Hits 1594 1594
Misses 102 102
Partials 31 31 Continue to review full report at Codecov.
|
|
||
final String dataPrepperMatchAttribute = "match"; | ||
final LogstashAttributesMappings mappings = mock(LogstashAttributesMappings.class); | ||
when(mappings.getMappedAttributeNames()).thenReturn(new HashMap<String, String>() {{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a pattern I've seen elsewhere in Data Prepper, but discourage. This is creating an anonymous class with a static initializer.
For this case, I recommend: Collections.singletonMap(..., ...)
.
Arrays.asList(Map.entry("message", "fake message regex 1"), Map.entry("other", "fake other regex"))); | ||
final LogstashAttribute matchMessageLogstashAttribute2 = prepareArrayTypeMatchLogstashAttribute("message", "fake message regex 2"); | ||
final List<LogstashAttribute> matchLogstashAttributes = Arrays.asList(matchMultiKeysLogstashAttribute, matchMessageLogstashAttribute2); | ||
final Map<String, Object> expectedPluginSettings = new HashMap<String, Object>() {{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As with my note below, I'd prefer to rework this. Since this is a test, you can use Java 11's Map.of
.
Map.of("message", Arrays..., "other", Collections...)
@@ -0,0 +1,93 @@ | |||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the incorrect header per #189.
It should be:
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/
import java.util.List; | ||
import java.util.Map; | ||
|
||
@SuppressWarnings("unchecked") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally try to keep these suppressions at tight as possible. I can tell you need it on mergeMatchAttributes
, and so I'd recommend adding the annotation to that method.
Arrays.asList(Map.entry("message", "fake message regex 1"), Map.entry("other", "fake other regex"))); | ||
final LogstashAttribute matchMessageLogstashAttribute2 = prepareArrayTypeMatchLogstashAttribute("message", "fake message regex 2"); | ||
final List<LogstashAttribute> matchLogstashAttributes = Arrays.asList(matchMultiKeysLogstashAttribute, matchMessageLogstashAttribute2); | ||
final Map<String, Object> expectedPluginSettings = new HashMap<String, Object>() {{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be named expectedMatchSettings
?
Signed-off-by: qchea <[email protected]>
Signed-off-by: qchea <[email protected]>
* main: Fixes the main branch from a renaming that happened when two related PRs were merged around the same time. (opensearch-project#583) Grpc Basic auth for Otel trace Source (opensearch-project#570) Fixed a NullPointerException which LogstashVisitor was throwing for attributes without a value. (opensearch-project#582) Update PipelineModel to fix serialization (opensearch-project#577) Log analytics getting started (opensearch-project#573) Support maps which are not present in the YAML, which Jackson appears to be treating differently from explicit nulls. opensearch-project#568 (opensearch-project#579) Add support for codeowners to repo (opensearch-project#578) Added logstash mapper which maps logstash configuration to pipeline m… (opensearch-project#575) Document OpenSearch Sink configuration parameters number_of_shards and number_of_replicas. (opensearch-project#562) Added instructions to build and run the Docker image locally. (opensearch-project#564) NPE bug fixes in the DefaultLogstashPluginAttributesMapper (opensearch-project#574) switch Path.of to Paths.get (opensearch-project#566) Fix PrepperState javadoc (opensearch-project#567) Signed-off-by: qchea <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks, this looks good! |
Signed-off-by: qchea [email protected]
Description
This PR adds the custom attribute mapper for grok.
Issues Resolved
#466
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.