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

Kubernetes module for metricbeat #3916

Merged
merged 2 commits into from Apr 10, 2017

Conversation

Projects
None yet
5 participants
@vjsamuel
Copy link
Contributor

commented Apr 5, 2017

First pass on integrating with kubelet.

@elasticmachine

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2017

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.

1 similar comment
@elasticmachine

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2017

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.

@@ -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"

This comment has been minimized.

Copy link
@exekias

exekias Apr 5, 2017

Member

probably these are spurious :)

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

removed

@vjsamuel vjsamuel force-pushed the vjsamuel:kubernetes_module branch 2 times, most recently from 63ca4f9 to 8a7933a Apr 6, 2017

@ruflin
Copy link
Collaborator

left a comment

I left some minor comments.

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

What is the default port?

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

done

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

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

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

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

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

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

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

done

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

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

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

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

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

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.

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

done.

"start_time": container.StartTime,

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

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 ...

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

done

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

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

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

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.

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

i see what your saying.

},

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 6, 2017

Collaborator

See comments above about schema and 2.x

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 6, 2017

Author Contributor

done

@vjsamuel vjsamuel force-pushed the vjsamuel:kubernetes_module branch 2 times, most recently from 4f27f0a to 463cf96 Apr 6, 2017

@ruflin

ruflin approved these changes Apr 7, 2017

Copy link
Collaborator

left a comment

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.
"container": {
"cpu": {
"usage": {
"corenanoseconds": 3305756719,

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 7, 2017

Collaborator

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

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

This comment has been minimized.

Copy link
@ruflin

ruflin Apr 7, 2017

Collaborator

What is that exactly?

This comment has been minimized.

Copy link
@vjsamuel

vjsamuel Apr 7, 2017

Author Contributor

@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"`
}

@vjsamuel vjsamuel force-pushed the vjsamuel:kubernetes_module branch from 463cf96 to 523a269 Apr 7, 2017

@vjsamuel vjsamuel force-pushed the vjsamuel:kubernetes_module branch from 523a269 to b923485 Apr 7, 2017

@andrewkroh

This comment has been minimized.

Copy link
Member

commented Apr 7, 2017

jenkins, test it

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

This comment has been minimized.

Copy link
@elasticmachine

elasticmachine Apr 7, 2017

Collaborator

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

@@ -0,0 +1,119 @@
package kubelet

type Summary struct {

This comment has been minimized.

Copy link
@elasticmachine

elasticmachine Apr 7, 2017

Collaborator

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

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

6 checks passed

CLA Commit author has signed the CLA
Details
codecov/patch 1.31% of diff hit (within 100% threshold of 61.28%)
Details
codecov/project 64.87% (+3.58%) compared to ee07419
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Build finished.
Details

@exekias exekias referenced this pull request Apr 10, 2017

Closed

Follow up kubelet module #3965

8 of 8 tasks complete

@vjsamuel vjsamuel deleted the vjsamuel:kubernetes_module branch Apr 10, 2017

athom added a commit to athom/beats that referenced this pull request Jan 25, 2018

Kubernetes module for metricbeat (elastic#3916)
* Kubernetes module for metricbeat

* Changing nanoseconds in schema of kubelet module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.