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

Bring back support for grok patterns containing named groups with underscore #2

Merged
merged 7 commits into from
Mar 25, 2019

Conversation

kmerz
Copy link

@kmerz kmerz commented Feb 28, 2019

Grok Patterns with named groups ((?<name0>test)) can contain a name with a underscore ((?<my_name>test)).

The java-grok library dropped the support unintentionally by moving the regex Engine from "named-regexp" to java (thekrakken#53).

To preserve the support this PR switches back from java to "named-regexp".

@kmerz kmerz added the bug label Feb 28, 2019
@kmerz kmerz requested a review from bernd February 28, 2019 13:55
@CLAassistant
Copy link

CLAassistant commented Feb 28, 2019

CLA assistant check
All committers have signed the CLA.


import io.krakens.grok.api.Converter.IConverter;

import com.google.code.regexp.Matcher;
import com.google.code.regexp.Pattern;

Copy link

Choose a reason for hiding this comment

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

The commit that changed to java.util.regex in the upstream repository (thekrakken@bd098c3) changed the following in this file:

image

With your change we are still calling GrokUtils.namedGroups(m, m.group()); instead of the previous m.namedGroups(). Is that correct or do we need to use m.namedGroups() again?

import java.util.regex.Matcher;

import com.google.code.regexp.Matcher;

Copy link

Choose a reason for hiding this comment

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

Same as above, do we need to use m.namedGroups() here?

-    Map<String, String> mappedw = this.match.namedGroups();
+    Map<String, String> mappedw = GrokUtils.namedGroups(this.match,this.subject);

@bernd bernd self-assigned this Mar 22, 2019
The test originally assumed that name0 is
the result with underscore.

I have no idea why I thought this.

The test now will test that a pattern with
underscore will be returned proper after
compilation
@bernd bernd merged commit 8739711 into repackaged2 Mar 25, 2019
@bernd bernd deleted the google-pattern-engine branch March 25, 2019 16:27
bernd added a commit to Graylog2/graylog2-server that referenced this pull request Mar 25, 2019
To support underscores ("_") in Grok match group names, we had to modify
the java-grok library to use the old regexp engine again.

See: graylog-labs/java-grok#2

This also adds a test for the Grok extractor to make sure that using
underscores works.

Fixes #5704
Fixes #5563
kmerz pushed a commit to Graylog2/graylog2-server that referenced this pull request Mar 26, 2019
* Switch back to a repackaged and fixed version of java-grok

To support underscores ("_") in Grok match group names, we had to modify
the java-grok library to use the old regexp engine again.

See: graylog-labs/java-grok#2

This also adds a test for the Grok extractor to make sure that using
underscores works.

Fixes #5704
Fixes #5563

* Fix GrokPatternService#extractPatternNames and add a test for it

* Add missing license header to GrokPatternServiceTest

* Add test for named group with underscore

Prior to this change, there was no test for named groups
with underscores in the FunctionSnippetsTest

This change enhances the grok() test to run with a
named group with underscore.
bernd added a commit to Graylog2/graylog2-server that referenced this pull request Mar 26, 2019
* Switch back to a repackaged and fixed version of java-grok

To support underscores ("_") in Grok match group names, we had to modify
the java-grok library to use the old regexp engine again.

See: graylog-labs/java-grok#2

This also adds a test for the Grok extractor to make sure that using
underscores works.

Fixes #5704
Fixes #5563

* Fix GrokPatternService#extractPatternNames and add a test for it

* Add missing license header to GrokPatternServiceTest

* Add test for named group with underscore

Prior to this change, there was no test for named groups
with underscores in the FunctionSnippetsTest

This change enhances the grok() test to run with a
named group with underscore.

(cherry picked from commit e642a41)
kmerz pushed a commit to Graylog2/graylog2-server that referenced this pull request Mar 26, 2019
…5807)

* Switch back to a repackaged and fixed version of java-grok

To support underscores ("_") in Grok match group names, we had to modify
the java-grok library to use the old regexp engine again.

See: graylog-labs/java-grok#2

This also adds a test for the Grok extractor to make sure that using
underscores works.

Fixes #5704
Fixes #5563

* Fix GrokPatternService#extractPatternNames and add a test for it

* Add missing license header to GrokPatternServiceTest

* Add test for named group with underscore

Prior to this change, there was no test for named groups
with underscores in the FunctionSnippetsTest

This change enhances the grok() test to run with a
named group with underscore.

(cherry picked from commit e642a41)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants