-
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
Named captures conversion to grok pattern_definitions format #586
Named captures conversion to grok pattern_definitions format #586
Conversation
Signed-off-by: Taylor Gray <[email protected]>
Signed-off-by: Taylor Gray <[email protected]>
…s format Signed-off-by: Taylor Gray <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #586 +/- ##
=========================================
Coverage 92.12% 92.12%
Complexity 569 569
=========================================
Files 71 71
Lines 1727 1727
Branches 144 144
=========================================
Hits 1591 1591
Misses 105 105
Partials 31 31 Continue to review full report at Codecov.
|
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.
Only minor nits. Not blocking
|
||
class GrokNamedCapturesUtil { | ||
|
||
private static final String namedCapturesRegex = "\\(\\?\\<(.+?)\\>(.+?)\\)"; |
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.
Nit: Could be all capitalized
} | ||
|
||
public Map<String, String> getMappedPatternDefinitions() { | ||
return mappedPatternDefinitions; |
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.
Nit: For access safety, I suggest return a view of the map, i.e. Collections.unmodifiableMap(mappedPatternDefinitions).
|
||
for (final Map.Entry<String, String> patternDefinition : result.getMappedPatternDefinitions().entrySet()) { | ||
assertThat(patternDefinition.getValue().equals(namedCapturesPattern), equalTo(true)); | ||
final String expectedResult = String.format("%%{%s:%s}", patternDefinition.getKey(), namedCapturesName); |
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 was expecting %{%s:%s}
but could be wrong.
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.
%%
is how you do a literal %
in String.format
} | ||
final String expectedResult = String.format("%s%%{%s:%s} %s %%{%s:%s}%s", randomPrefix, patternDefinitionNames.get(0), namedCapturesNames.get(0), randomMiddle, | ||
patternDefinitionNames.get(1), namedCapturesNames.get(1), randomSuffix); | ||
assertThat(result.getMappedRegex().equals(expectedResult), equalTo(true)); |
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 find it better to use the following form because you get better output when the assertion fails:
assertThat(result.getMappedRegex(), equalTo(expectedResult));
Description
This PR contains a utility class for taking any regex string and pulling out the named captures groups from it for the grok pattern_definition format. It then replaces the original named captures group with grok syntax
%{SYNTAX:SEMANTIC}
. It creates random pattern names to go with thePATTERN_NAME regex
format of pattern definition declarationIssues Resolved
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.