Fetches control group resource information by tylersmalley · Pull Request #10402 · elastic/kibana

tylersmalley · 2017-02-16T17:21:22Z

These are made available to the status API api/status and kbnServer.metrics.

jbudz · 2017-02-16T18:39:39Z

Test failure looks legitimate, I'm guessing we need to wait for the promise to resolve on previous status tests

jbudz · 2017-02-21T19:45:21Z

do we need to update this to match? elastic/elasticsearch@00a8b87

jbudz · 2017-02-21T19:51:07Z

is it possible to check for the files once, and if they don't exist stop checking?

Now preventing lookups when we determine there is no cgroups information. 655f54b

tsullivan

Looks like this grabs the cgroup info from /sys/fs/cgroup, unfortunately I think this does need to be tested from inside a Docker container as the ES team found there are inconsistencies in where Docker publishes the cgroup statistics: elastic/elasticsearch#22757

tylersmalley · 2017-03-27T15:30:56Z

@tsullivan - I pushed a fix for the docker cgroup path.

Here is a docker image on Dockerhub to help with testing:

tylersmalley/kibana:6.0.0-alpha1-SNAPSHOT-1ae1a6a

You can limit the CPU to exposed throttled metrics by passing --cpus=0.1 to docker run

And here are snapshots of the PR:

https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-amd64.deb
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-darwin-x86_64.tar.gz
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-i386.deb
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-i686.rpm
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-linux-x86.tar.gz
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-linux-x86_64.tar.gz
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-windows-x86.zip
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-1ae1a6a/kibana/kibana-6.0.0-alpha1-SNAPSHOT-x86_64.rpm

When running, you can apply cgroup constraints to a process/group. To do so, run systemd-cgls to find the user slice which contains kibana. You can then use it to set CPUQuota (example: systemctl set-property user.slice CPUQuota=100%)

tsullivan · 2017-03-28T16:30:19Z

I would recommend using the radix parameter of parseInt, because without it behavior could be unpredictable.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt

tsullivan · 2017-03-28T16:30:39Z

radix parameter on these parseInts

tsullivan · 2017-03-28T16:33:32Z

Thanks for providing the Docker images to test with - that was extremely helpful.

I left just one comment about something that popped up in a few places. I'm surprised that radix for parseInt thing isn't part of our lint rules 😺

jbudz · 2017-03-28T19:41:27Z

Rebasing should fix the test failures

jbudz · 2017-03-28T19:38:10Z

do you think there's any need for the path to be configurable? ie instead of a bool, cpu.cgroup.path = Joi.string()

These configuration options align with logstash/ES outlined here: elastic/logstash#6797 (comment)

👍 for consistency. it's not clear to me from the thread if override is meant to be a bool, the current (eventually deprecated) implementation in es looks like a string. if i'm missing something feel free to ignore.

You're right, good find. https://github.com/elastic/elasticsearch-docker/blob/master/build/elasticsearch/bin/es-docker#L37

Will update

jbudz · 2017-03-28T19:46:11Z

how would you feel about logging this error instead of throwing? if something comes up we can still return non c-group stats

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

This was addressed in elastic/elasticsearch#23219 Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Docker cgroups are mounted in the wrong place (i.e., inconsistently with /proc/self/cgroup). This commit adds an undocumented hack for working around, for now. Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley · 2017-04-05T18:36:15Z

Addressed feedback and pushed new builds for testing:

DockerHub: tylersmalley/kibana:6.0.0-alpha1-SNAPSHOT-5812d7b

Example: docker run -p 5601:5601 -e "ELASTICSEARCH_URL=http://172.16.0.63:9200" --cpus=0.1 tylersmalley/kibana:6.0.0-alpha1-SNAPSHOT-5812d7b

https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-amd64.deb
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-darwin-x86_64.tar.gz
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-i386.deb
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-i686.rpm
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-linux-x86.tar.gz
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-linux-x86_64.tar.gz
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-windows-x86.zip
https://download.elastic.co/kibana/staging/6.0.0-alpha1-SNAPSHOT-5812d7b/kibana/kibana-6.0.0-alpha1-SNAPSHOT-x86_64.rpm

jbudz

Nice. One inline question below. And can we add docs for the new configuration values?

Otherwise LGTM:

cgroup: {
  cpuacct: {
    control_group: "/",
    usage_nanos: 42023428539
  },
  cpu: {
    control_group: "/",
    cfs_period_micros: 100000,
    cfs_quota_micros: -1,
    stat: {
      number_of_elapsed_periods: 0,
      number_of_times_throttled: 0,
      time_throttled_nanos: 0
    }
  }
}

jbudz · 2017-04-05T19:15:33Z

-export function getMetrics({ event, config }) {
+export async function getMetrics(event, config, server) {
  const port = config.get('server.port');
  const timestamp = new Date().toISOString();


do you think we should set timestamp after cgroup stats have been read? along the same lines, do you think we should only getMetrics once the previous getMetrics call is done so they don't queue up for frequent ops events?

Having it right at the ops event is probably best as it's actually involved in aggregations (request count, etc). Capturing cgroups is also a very quick operation in that it's reading most of the files in parallel and they aren't actually on disk. We should revisit when we add CPU percentage.

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley · 2017-04-05T20:02:57Z

@jbudz documented configuration options - placed near existing status config.

tylersmalley · 2017-04-06T15:23:40Z

@tsullivan - mind giving another look?

pickypg

I tested this successfully on Windows (no Cgroup data, but it didn't explode either). I just have some questions about the settings.

pickypg · 2017-04-10T19:58:07Z

 The minimum value is 100.
 `status.allowAnonymous`:: *Default: false* If authentication is enabled, setting this to `true` allows
 unauthenticated users to access the Kibana server status API and status page.
+`cpu.cgroup.path.override`:: Override for cgroup cpu path


Could we show an example of this being overridden?

pickypg · 2017-04-10T19:59:57Z


+  cpu: Joi.object({
+    cgroup: Joi.object({
+      path: Joi.object({


I don't feel like we normally specific "override" as the field to override, rather just cpu.cgroup.path would be set or unset (default). Specifically I'm looking at examples like server.basePath, plugins.paths, and path.data.

The name was discussed here for Logstash/ES. While I agree we could omit "override" it maintains consistency with the other projects.

Ah that's unfortunate, but I guess it's intentional to really show that you're going against defaults.

pickypg

LGTM

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Adds control group data to status API and kbnServer.metrics

tylersmalley · 2017-04-11T19:05:42Z

5.x: b8bb212

Adds control group data to status API and kbnServer.metrics

tylersmalley added Team:Operations Kibana-Operations Team review v5.4.0 v6.0.0 labels Feb 16, 2017

tylersmalley requested a review from jbudz February 16, 2017 17:21

jbudz reviewed Feb 21, 2017

View reviewed changes

tylersmalley requested review from pickypg and tsullivan February 23, 2017 16:46

tsullivan requested changes Mar 15, 2017

View reviewed changes

tylersmalley force-pushed the status-api-cgroup branch 3 times, most recently from 1ae1a6a to ac70577 Compare March 27, 2017 15:24

tsullivan reviewed Mar 28, 2017

View reviewed changes

Comment thread src/server/status/cgroup.js Outdated

Copy link
Copy Markdown

Member

tsullivan Mar 28, 2017

Choose a reason for hiding this comment

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

radix parameter on these parseInts

jbudz reviewed Mar 28, 2017

View reviewed changes

tylersmalley added 6 commits March 28, 2017 16:39

Adds control group data to status API and kbnServer.metrics

bebddda

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Fixes control group issue

1a66767

This was addressed in elastic/elasticsearch#23219 Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Await call to getMetrics()

ea394a2

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Prevent IO if cgroups is not available

378386e

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Adds hack for docker cgroup path

bfcde14

Docker cgroups are mounted in the wrong place (i.e., inconsistently with /proc/self/cgroup). This commit adds an undocumented hack for working around, for now. Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Adds radix to parseInt

3d1caca

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley force-pushed the status-api-cgroup branch from c61e7fd to 3d1caca Compare March 28, 2017 23:40

Override should be a string and we should log errors

71a60b1

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley force-pushed the status-api-cgroup branch from 5812d7b to 71a60b1 Compare April 5, 2017 17:51

tylersmalley mentioned this pull request Apr 5, 2017

Overrides cgroup path elastic/kibana-docker#23

Merged

jbudz reviewed Apr 5, 2017

View reviewed changes

tylersmalley added 2 commits April 5, 2017 12:46

Use stub to remove duplication

8bb236b

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

Adds documentation for cgroup override

3ba904d

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley removed the request for review from pickypg April 6, 2017 15:23

jbudz approved these changes Apr 6, 2017

View reviewed changes

pickypg self-requested a review April 10, 2017 19:32

pickypg reviewed Apr 10, 2017

View reviewed changes

pickypg approved these changes Apr 10, 2017

View reviewed changes

Updates description for cgroup override

cccef8a

Signed-off-by: Tyler Smalley <tyler.smalley@elastic.co>

tylersmalley merged commit 006fae0 into elastic:master Apr 11, 2017

tylersmalley deleted the status-api-cgroup branch April 11, 2017 17:20

tylersmalley added a commit to tylersmalley/kibana that referenced this pull request Apr 11, 2017

Fetches control group resource information (elastic#10402)

dffb9c9

Adds control group data to status API and kbnServer.metrics

tylersmalley mentioned this pull request Apr 11, 2017

[5.x] Fetches control group resource information (#10402) #11163

Merged

tylersmalley added a commit that referenced this pull request Apr 11, 2017

Fetches control group resource information (#10402) (#11163)

b8bb212

Adds control group data to status API and kbnServer.metrics

tylersmalley added backported and removed backported labels Apr 11, 2017

elasticmachine mentioned this pull request Apr 25, 2018

Kibana monitoring agent should collect cgroup info #18224

Closed

2 tasks

Conversation

tylersmalley commented Feb 16, 2017

Uh oh!

jbudz commented Feb 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsullivan left a comment

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Mar 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tsullivan commented Mar 28, 2017

Uh oh!

jbudz commented Mar 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Apr 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jbudz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Apr 5, 2017

Uh oh!

tylersmalley commented Apr 6, 2017

Uh oh!

pickypg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pickypg left a comment

Choose a reason for hiding this comment

Uh oh!

tylersmalley commented Apr 11, 2017 • edited by epixa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tylersmalley commented Apr 5, 2017 •

edited

Loading

tylersmalley commented Apr 11, 2017 •

edited by epixa

Loading