Kubernetes module for metricbeat #3916

vjsamuel · 2017-04-05T08:39:57Z

First pass on integrating with kubelet.

elasticmachine · 2017-04-05T08:42:03Z

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

elasticmachine · 2017-04-05T08:42:10Z

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

exekias · 2017-04-05T16:01:50Z

metricbeat/include/list.go

@@ -28,6 +28,8 @@ import (
 	_ "github.com/elastic/beats/metricbeat/module/docker/info"
 	_ "github.com/elastic/beats/metricbeat/module/docker/memory"
 	_ "github.com/elastic/beats/metricbeat/module/docker/network"
+	_ "github.com/elastic/beats/metricbeat/module/dropwizard"
+	_ "github.com/elastic/beats/metricbeat/module/dropwizard/collector"


probably these are spurious :)

ruflin

I left some minor comments.

ruflin · 2017-04-06T06:47:29Z

metricbeat/metricbeat.full.yml

+  metricsets: ["node","container","volume","pod","system"]
+  enabled: true
+  period: 10s
+  hosts: ["localhost"]


What is the default port?

ruflin · 2017-04-06T06:47:41Z

metricbeat/metricbeat.full.yml

@@ -179,6 +179,14 @@ metricbeat.modules:
  #username: ""
  #password: ""

+#------------------------------- kubelet Module ------------------------------
+- module: kubelet


Should be commented out so it is not enabled by default.

ruflin · 2017-04-06T06:48:06Z

metricbeat/metricbeat.yml

@@ -46,6 +46,14 @@ metricbeat.modules:
  period: 10s
  processes: ['.*']

+#------------------------------- kubelet Module ------------------------------


You can use short_config: false in the module fields.yml so it is not added to the short config.

ruflin · 2017-04-06T06:49:09Z

metricbeat/module/dropwizard/collector/collector.go

@@ -0,0 +1,96 @@
+package collector


dropwizard? Is that here intentional? Sounds like an other potential module for the future :-)

removed. added it by mistake to this PR. it is the next one ;)

ruflin · 2017-04-06T06:51:00Z

metricbeat/module/kubelet/_meta/fields.yml

+- key: kubelet
+  title: "kubelet"
+  description: >
+    kubelet Module


Can you add []beta flag and short_config: false here? See https://github.com/elastic/beats/blob/master/metricbeat/module/docker/_meta/fields.yml#L7 for an example.

ruflin · 2017-04-06T06:52:50Z

metricbeat/module/kubelet/container/data.go

+				"start_time": container.StartTime,
+
+				"cpu": common.MapStr{
+					"usage.nanocores":       container.CPU.UsageNanoCores,


That should be "usage": common.MapStr{ "nanocores" }. The reason is otherwise it will break with elasticsearch 2.x. We often use `common.MapStr.Put(...) in these cases as it does it automatically, but not sure how we could use this here ...

ruflin · 2017-04-06T06:53:54Z

metricbeat/module/kubelet/container/data.go

+		for _, container := range pod.Containers {
+			containerEvent := common.MapStr{
+				mb.ModuleData: common.MapStr{
+					"pod": common.MapStr{


Seems like the schema package could be used here to do the mapping. Can also be done later.

schema wouldnt work because the input interface coming from the HTTP has arrays.

I see. What I did in other metricsets is using multiple schemas. Means doing the array "manually" and then under it having schemas. But we can look into it after the first version.

i see what your saying.

ruflin · 2017-04-06T06:54:46Z

metricbeat/module/kubelet/node/data.go

+		},
+
+		"memory": common.MapStr{
+			"available.bytes":  node.Memory.AvailableBytes,


See comments above about schema and 2.x

ruflin

I had a look again at the code and I think most of the basic things are there that we need. To split future work I suggest we move forward with this PR and create a follow up Github issue to track the open tasks. Some of the tasks I currently see:

Complete fields.yml files with all fields
Add system tests to check if all fields are documented
Add tests -> this is trickiers, as probably can't run a kubelet easily to do integration tests? Using a mock golang http service could be used to just returns predefined json docs?
Check fields again if they follow the conventions
Add more documentation on how to use it etc.
Switch to usage of schema where possible. This should also simplify the testing.
Check that all beta flags exist and work.

ruflin · 2017-04-07T11:36:31Z

metricbeat/module/kubelet/container/_meta/data.json

+        "container": {
+            "cpu": {
+                "usage": {
+                    "corenanoseconds": 3305756719,


Is this nano seconds? If yes, we should use .ns: https://www.elastic.co/guide/en/beats/libbeat/5.1/event-conventions.html

ruflin · 2017-04-07T11:36:40Z

metricbeat/module/kubelet/container/_meta/data.json

+            "cpu": {
+                "usage": {
+                    "corenanoseconds": 3305756719,
+                    "nanocores": 5992


What is that exactly?

@ruflin

// CPUStats contains data about CPU usage. type CPUStats struct { // The time at which these stats were updated. Time metav1.Time `json:"time"` // Total CPU usage (sum of all cores) averaged over the sample window. // The "core" unit can be interpreted as CPU core-nanoseconds per second. // +optional UsageNanoCores *uint64 `json:"usageNanoCores,omitempty"` // Cumulative CPU usage (sum of all cores) since object creation. // +optional UsageCoreNanoSeconds *uint64 `json:"usageCoreNanoSeconds,omitempty"` }

andrewkroh · 2017-04-07T22:34:26Z

jenkins, test it

elasticmachine · 2017-04-07T23:01:58Z

metricbeat/module/kubelet/doc.go

@@ -0,0 +1,4 @@
+/*


[golint] _{reported by reviewdog 🐶}
package comment should be of the form "Package kubelet ..."

elasticmachine · 2017-04-07T23:01:58Z

metricbeat/module/kubelet/types.go

@@ -0,0 +1,119 @@
+package kubelet
+
+type Summary struct {


[golint] _{reported by reviewdog 🐶}
exported type Summary should have comment or be unexported

ruflin added in progress Pull request is currently in progress. Metricbeat Metricbeat module review labels Apr 5, 2017

exekias reviewed Apr 5, 2017

View reviewed changes

vjsamuel force-pushed the kubernetes_module branch 2 times, most recently from 63ca4f9 to 8a7933a Compare April 6, 2017 04:57

ruflin reviewed Apr 6, 2017

View reviewed changes

vjsamuel force-pushed the kubernetes_module branch 2 times, most recently from 4f27f0a to 463cf96 Compare April 6, 2017 22:07

ruflin approved these changes Apr 7, 2017

View reviewed changes

Kubernetes module for metricbeat

7ecc527

vjsamuel force-pushed the kubernetes_module branch from 463cf96 to 523a269 Compare April 7, 2017 15:44

Changing nanoseconds in schema of kubelet module

b923485

vjsamuel force-pushed the kubernetes_module branch from 523a269 to b923485 Compare April 7, 2017 21:40

elasticmachine reviewed Apr 7, 2017

View reviewed changes

exekias merged commit 7dadfe7 into elastic:master Apr 10, 2017

exekias removed in progress Pull request is currently in progress. review labels Apr 10, 2017

exekias mentioned this pull request Apr 10, 2017

Follow up kubelet module #3965

Closed

8 tasks

vjsamuel deleted the kubernetes_module branch April 10, 2017 07:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubernetes module for metricbeat #3916

Kubernetes module for metricbeat #3916

vjsamuel commented Apr 5, 2017

elasticmachine commented Apr 5, 2017

elasticmachine commented Apr 5, 2017

exekias Apr 5, 2017

vjsamuel Apr 6, 2017

ruflin left a comment

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin Apr 6, 2017

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin Apr 6, 2017

vjsamuel Apr 6, 2017

ruflin left a comment

ruflin Apr 7, 2017

ruflin Apr 7, 2017

vjsamuel Apr 7, 2017

andrewkroh commented Apr 7, 2017

elasticmachine Apr 7, 2017

elasticmachine Apr 7, 2017

Kubernetes module for metricbeat #3916

Kubernetes module for metricbeat #3916

Conversation

vjsamuel commented Apr 5, 2017

elasticmachine commented Apr 5, 2017

elasticmachine commented Apr 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewkroh commented Apr 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment