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

Send float fields as float values to Elasticsearch #2627

Merged
merged 1 commit into from
Oct 5, 2016

Conversation

monicasarbu
Copy link
Contributor

@monicasarbu monicasarbu commented Sep 23, 2016

This PR tries to remove the necessity to define the mapping before inserting the data into Elasticsearch, by making sure that float fields are always sent as float values even if they are not flows. For example 5 will be sent as 5.00000, so Elasticsearch defines it as float.

With this change, it's not needed to load the template in advance, before inserting the data into Elasticsearch.

NOTE: This also affects processors.

@monicasarbu monicasarbu added in progress Pull request is currently in progress. discuss Issue needs further discussion. review labels Sep 23, 2016
@ruflin
Copy link
Contributor

ruflin commented Oct 3, 2016

I suggest we backport this to 5.0 as soon as the tests pass as I don't think this is a breaking change. As we discussed it does not prevent people from still loading the dashboard, but it is a nice fallback.

@ruflin ruflin added the needs_backport PR is waiting to be backported to other branches. label Oct 3, 2016
@monicasarbu monicasarbu force-pushed the autodetect_float_values branch from 49539f9 to 5b0b8c2 Compare October 4, 2016 09:36
@monicasarbu monicasarbu removed the in progress Pull request is currently in progress. label Oct 4, 2016
@monicasarbu monicasarbu force-pushed the autodetect_float_values branch from d4e4b08 to 38e7fae Compare October 4, 2016 12:11
@@ -134,7 +134,7 @@ def test_dropevent_with_complex_condition(self):
output = self.read_output(
required_fields=["@timestamp", "type"],
)
assert len(output) == 1
assert len(output) >= 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change somehow related to the changes above or just a flaky test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

flaky test, as it's variable how many events are sent out.

@ruflin
Copy link
Contributor

ruflin commented Oct 4, 2016

LGTM

@ruflin ruflin merged commit bec6851 into elastic:master Oct 5, 2016
@monicasarbu monicasarbu deleted the autodetect_float_values branch October 5, 2016 11:32
monicasarbu added a commit to monicasarbu/beats that referenced this pull request Oct 5, 2016
tsg pushed a commit that referenced this pull request Oct 5, 2016
@monicasarbu monicasarbu removed the needs_backport PR is waiting to be backported to other branches. label Jan 31, 2017
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs further discussion. review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants