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

Export cpu usage in percentage by default #1705

Merged

Conversation

monicasarbu
Copy link
Contributor

@monicasarbu monicasarbu commented May 23, 2016

With this PR, I did the minimal changes that Topbeat and Metricbeat to export (by default) the CPU usage statistics in percentage instead of CPU ticks. The percentages are:

  • cpu.iowait_p, cpu.idle_p, cpu.irq_p, cpu.nice_p, cpu.softirq_p, cpu.steal_p, cpu.system_p and cpu.user_p.
  • proc.cpu.total_p

If the user is interested in exporting the CPU usage in ticks , he/she needs to set the configuration option cpu_usage_in_ticks to true. The following values are exported in addition to the CPU usage in percentage:

  • cpu.iowait, cpu.idle, cpu.irq, cpu.nice, cpu.softirq, cpu.steal
  • proc.cpu.user, proc.cpu.system, proc.cpu.total
  • core.iowait, core.idle, core.irq, core.nice, core.softirq, core.steal

We don't need to change the Topbeat dashboard as the values that we are making optional, are not used in the dashboard.

cc-ed: @tsg @simianhacker @ruflin @andrewkroh

@monicasarbu monicasarbu force-pushed the export_cpu_usage_in_percentage_by_default branch from 394127b to 818424b Compare May 23, 2016 17:44
@monicasarbu monicasarbu added review and removed review labels May 23, 2016
@monicasarbu monicasarbu force-pushed the export_cpu_usage_in_percentage_by_default branch from b92e6d4 to cb40022 Compare May 23, 2016 20:36
- name: irq
type: integer
description: >
The amount of CPU time spent servicing and handling hardware interrupts.

- name: irq_p
type: integer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be float. I am fixing it.

@ruflin
Copy link
Member

ruflin commented May 23, 2016

In general LGTM. Somme suggestions/comments:

  • We should group the ticks together under the namespace ticks.
  • Having ticks grouped together, allows us to call the current user_p just user. Otherwise I would call it user_pct, at least in metricbeat
  • I'm not sure if you run the make collect script. Please check if that overwrites stuff again.
  • Rename the really long config option to just ticks.

@monicasarbu monicasarbu force-pushed the export_cpu_usage_in_percentage_by_default branch from 9b9b1c8 to 6f76d43 Compare May 24, 2016 12:52
@monicasarbu
Copy link
Contributor Author

monicasarbu commented May 24, 2016

I updated the PR with all the comments.

@ruflin I kept user_p for now, for alpha3 as there are quite many changes for it.

@monicasarbu monicasarbu force-pushed the export_cpu_usage_in_percentage_by_default branch 2 times, most recently from 4bee927 to 4cd909a Compare May 24, 2016 14:38
@monicasarbu
Copy link
Contributor Author

jenkins, test it

@monicasarbu monicasarbu force-pushed the export_cpu_usage_in_percentage_by_default branch from 4cd909a to 9ef54d0 Compare May 24, 2016 14:41
@@ -13,10 +13,11 @@ topbeat.procs: [

# Statistics to collect (all enabled by default)
topbeat.stats:
system: {{ "false" if system_stats == false else "true" }}
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this is 4 spaces now. Does it still work? Is there a need to change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think it's working. Adding more spaces at each line wasn't on purpose.

@ruflin
Copy link
Member

ruflin commented May 24, 2016

LGTM

@monicasarbu
Copy link
Contributor Author

jenkins, test it

@monicasarbu monicasarbu force-pushed the export_cpu_usage_in_percentage_by_default branch from d16500a to 4419741 Compare May 24, 2016 16:42
The following changes are included in the PR:
- Exports CPU usage in percentage (default behaviour)
- Add `cpu_ticks` option in the system module of Metricbeat and Topbeat. If it's enabled, it exports the CPU
  usage in ticks together with the values in percentages. By default is false.
- Export CPU stats per core in `type=core` documents instead of `type=system` documents.
@monicasarbu monicasarbu force-pushed the export_cpu_usage_in_percentage_by_default branch from 4419741 to 142c71b Compare May 24, 2016 16:44
@monicasarbu
Copy link
Contributor Author

jenkins, test it

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.

5 participants