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

Fix mapping and race condition in sql module #15738

Merged
merged 6 commits into from
Jan 22, 2020

Conversation

jsoriano
Copy link
Member

@jsoriano jsoriano commented Jan 22, 2020

Mapping was defined at the metricset level, but it should be defined at
the module level.

Add system test to earlier detect this kind of issues.

Also add validation so driver and sql_query options cannot be empty.

Fetch was reusing some maps when building the event, what was
causing mixed data in events and panics on queries with lots of rows.

Fixes #15736

Mapping was defined at the metricset level, but it should be defined at
the module level.

Add system test to earlier check documented fields.

Fixes elastic#15736
@jsoriano jsoriano changed the title Fix mapping in sql module Fix mapping and race condition in sql module Jan 22, 2020
Copy link
Contributor

@narph narph left a comment

Choose a reason for hiding this comment

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

LGTM

@jsoriano jsoriano added needs_backport PR is waiting to be backported to other branches. v7.6.0 labels Jan 22, 2020
+
--
Non-numeric values collected.
Non-numeric values collected. - release: beta
Copy link
Contributor

Choose a reason for hiding this comment

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

something smells bad here

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I have needed a while to find the proper combination of spaces in the yamls, it should be fixed now.

Prometheus and statsd modules have the same problem, I will open a different PR to fix them.

Copy link
Member Author

Choose a reason for hiding this comment

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

PR for the same change in other modules #15742

@jsoriano jsoriano merged commit badb2cb into elastic:master Jan 22, 2020
@jsoriano jsoriano deleted the sql-fields-system-test branch January 22, 2020 15:53
jsoriano added a commit to jsoriano/beats that referenced this pull request Jan 22, 2020
Mapping was defined at the metricset level, but it should be defined at
the module level.

Add system test to earlier detect this kind of issues.

Also add validation so driver and sql_query options cannot be empty.

Fetch was reusing some maps when building the event, what was
causing mixed data in events and panics on queries with lots of rows.

Fixes elastic#15736

(cherry picked from commit badb2cb)
@jsoriano jsoriano added v7.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Jan 22, 2020
jsoriano added a commit to jsoriano/beats that referenced this pull request Jan 22, 2020
Mapping was defined at the metricset level, but it should be defined at
the module level.

Add system test to earlier detect this kind of issues.

Also add validation so driver and sql_query options cannot be empty.

Fetch was reusing some maps when building the event, what was
causing mixed data in events and panics on queries with lots of rows.

Fixes elastic#15736

(cherry picked from commit badb2cb)
jsoriano added a commit that referenced this pull request Jan 22, 2020
Mapping was defined at the metricset level, but it should be defined at
the module level.

Add system test to earlier detect this kind of issues.

Also add validation so driver and sql_query options cannot be empty.

Fetch was reusing some maps when building the event, what was
causing mixed data in events and panics on queries with lots of rows.

Fixes #15736

(cherry picked from commit badb2cb)
jsoriano added a commit that referenced this pull request Jan 23, 2020
Mapping was defined at the metricset level, but it should be defined at
the module level.

Add system test to earlier detect this kind of issues.

Also add validation so driver and sql_query options cannot be empty.

Fetch was reusing some maps when building the event, what was
causing mixed data in events and panics on queries with lots of rows.

Fixes #15736

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

Successfully merging this pull request may close these issues.

Metricbeat sql module: error mapping the query property
3 participants