Skip to content

Enable grok processor to support long, double and boolean #27896

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 20, 2017
Merged

Enable grok processor to support long, double and boolean #27896

merged 1 commit into from
Dec 20, 2017

Conversation

@kiawin
Copy link
Contributor

@kiawin kiawin commented Dec 19, 2017

It would be useful to allow grok processor to support long, double, and boolean type.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 19, 2017

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@martijnvg martijnvg requested a review from talevy December 20, 2017 08:17
@martijnvg
Copy link
Member

martijnvg commented Dec 20, 2017

@elasticmachine test this please

@martijnvg martijnvg removed the request for review from talevy December 20, 2017 08:44
@talevy talevy added the :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP label Dec 20, 2017
@talevy
Copy link
Contributor

talevy commented Dec 20, 2017

thanks @kiawin, Looks good!

@talevy talevy merged commit 47eefbe into elastic:master Dec 20, 2017
@talevy talevy added the v7.0.0 label Dec 20, 2017
martijnvg added a commit that referenced this pull request Dec 21, 2017
@martijnvg
Merge remote-tracking branch 'es/master' into ccr
c1a124c 
* es/master: (45 commits)
  Adapt scroll rest test after backport. relates #27842
  Move early termination based on index sort to TopDocs collector (#27666)
  Upgrade beats templates that we use for bwc testing. (#27929)
  ingest: upgraded ingest geoip's geoip2's dependencies.
  [TEST] logging for update by query test #27820
  Add elasticsearch-nio jar for base nio classes (#27801)
  Use full profile on JDK 10 builds
  Require Gradle 4.3
  Enable grok processor to support long, double and boolean (#27896)
  Add unreleased v6.1.2 version
  TEST: reduce blob size #testExecuteMultipartUpload
  Check index under the store metadata lock (#27768)
  Fixes DocStats to not report index size < -1 (#27863)
  Fixed test to be up to date with the new database files.
  Upgrade to Lucene 7.2.0. (#27910)
  Disable TestZenDiscovery in cloud providers integrations test
  Use `_refresh` to shrink the version map on inactivity (#27918)
  Make KeyedLock reentrant (#27920)
  ingest: Upgraded the geolite2 databases.
  [Test] Fix IndicesClientDocumentationIT (#27899)
  ...
@talevy
Copy link
Contributor

talevy commented Dec 21, 2017

This is pending a backport to 6.x because we'd like to see these changes reflected in Logstash's implementation of Grok. I plan to push a PR for that soon

@kiawin kiawin deleted the grok-processor-support-long-double-boolean branch December 22, 2017 06:47
@kiawin kiawin restored the grok-processor-support-long-double-boolean branch December 23, 2017 23:47
martijnvg added a commit that referenced this pull request Apr 5, 2018
@martijnvg
Removed tests that got added during a cherry-pick conflict.
c4f16eb 
These tests can run on master only. See #27896
@jpountz
Copy link
Contributor

jpountz commented Jan 29, 2019

@talevy Is the backport pending label still relevant?

@talevy
Copy link
Contributor

talevy commented Jan 29, 2019

:shame: I did not do a good job following up on this. It would be a simple backport to 6.7, but I waited
because I intended to make sure we have feature-parity in Logstash.

@jakelandis what do you think? Would you think this should go into 6.x?

@jakelandis
Copy link
Contributor

jakelandis commented Jan 30, 2019

@talevy - I think it is good to go back to 6.x. Logstash grok uses .to_i and .to_f to coerce the values, which transparently handle values larger then Java's Integer/Float. I believe support for boolean is the only difference this introduces (and difference are OK).

@talevy
Copy link
Contributor

talevy commented Jan 30, 2019

thanks @jakelandis. I'll go ahead and backport to 6.x!

and thanks again @jpountz for bouncing this back up!

talevy added a commit to talevy/elasticsearch that referenced this pull request Jan 30, 2019
@talevy
Enable grok processor to support long, double and boolean
06c86b5 
backport of elastic#27896.
@talevy talevy mentioned this pull request Jan 30, 2019
Merged
@talevy
Copy link
Contributor

talevy commented Jan 30, 2019

I opened #38000 as a copy-paste job from this PR

talevy added a commit that referenced this pull request Jan 30, 2019
@talevy
Enable grok processor to support long, double and boolean (#38000)
b13ae57 
Adds support for long,double,boolean coercion in grok expression.

backport of #27896.

note: the original commit contents was changed to the point
that cherry-picking/merging was more complicated than the
commit itself due to directory structure changes.

this represents the work that @kiawin did in the original PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >feature v6.7.0 v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@kiawin @elasticmachine @martijnvg @talevy @jpountz @jakelandis @colings86