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 system process CPU ticks field mapping #7230

Merged
merged 2 commits into from
Jun 1, 2018

Conversation

andrewkroh
Copy link
Member

Enabling process.include_cpu_ticks: true would cause an Elasticsearch mapping conflict because the .ticks fields for user and system were not defined in the fields.yml.

@andrewkroh andrewkroh added bug review Metricbeat Metricbeat needs_backport PR is waiting to be backported to other branches. labels May 31, 2018
Enabling `process.include_cpu_ticks: true` would cause an Elasticsearch mapping conflict because the `.ticks` fields for user and system were not defined in the fields.yml.
@andrewkroh andrewkroh force-pushed the bugfix/mb/system-process-ticks branch from dd006dd to 7f33134 Compare May 31, 2018 19:58
@ctindel
Copy link
Contributor

ctindel commented May 31, 2018

Shouldn't we add some tests that expose the original issue to make sure this doesn't happen again?

By enabling the feature we will automatically be ensuring that the fields
in the event are also in the fields.yml.
@andrewkroh
Copy link
Member Author

@ctindel I was thinking the same thing. I updated the test case for the process metricset. It ran the updated test case without the fix and it failed (e.g. Exception: Key 'system.process.cpu.system.ticks' found in event is not documented!).

@ctindel
Copy link
Contributor

ctindel commented May 31, 2018

Well done!

@ctindel
Copy link
Contributor

ctindel commented May 31, 2018

@andrewkroh do you know which point release this will ship in?

@ruflin ruflin merged commit 93eedf3 into elastic:master Jun 1, 2018
@ruflin
Copy link
Contributor

ruflin commented Jun 1, 2018

@andrewkroh I did a rebase merge on this as it had 2 clean commits with description. But after merging I realised it would have been better as 1 commit because of the backport :-(

@andrewkroh andrewkroh removed the needs_backport PR is waiting to be backported to other branches. label Jun 1, 2018
@tsg tsg added v6.3.1 and removed v6.3.0 labels Jun 25, 2018
@andrewkroh andrewkroh deleted the bugfix/mb/system-process-ticks branch July 27, 2018 14:58
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.

4 participants