Metricbeat: Collect accumulated docker network metrics #7253

jsoriano · 2018-06-04T15:27:34Z

Collect accumulated docker network metrics and mark previous metrics as deprecated.

ruflin

Needs rebase

ruflin · 2018-06-05T07:43:36Z

metricbeat/module/docker/network/_meta/fields.yml

+          type: long
+          description: >
+            Total number of outgoing packets.
+    - name: inbound


To follow ECS, should it be on the root level? Meaning having it under network.inbound.bytes ?

Umm, you are right, I was misinterpreting ECS for this case, would it be ok to have this without the docker preffix?

If the content of the field matches ECS I would say yes.

jsoriano · 2018-06-11T09:17:12Z

@ruflin I have done the changes for more proper ECS, this would probably require its own PR, but lets discuss all together before.

jsoriano · 2018-06-11T09:19:59Z

libbeat/_meta/fields.ecs-ext.yml

+- key: ecs-ext
+  title: ECS extensions
+  description: >
+    Contains extensions to ECS (https://github.com/elastic/ecs)


I have added this for common fields that are not (yet) in ECS, but they could be in the future. Changes here should be followed by discussions/PRs in ECS repo.
We could also use the commons file for that, but I think that it's fine to leave the common file for things needed by beats but that don't make sense in ECS.

jsoriano · 2018-06-11T09:21:40Z

libbeat/_meta/fields.ecs.yml

@@ -0,0 +1,55 @@
+- key: ecs


This file should probably be automatically generated from https://github.com/elastic/ecs/tree/master/schemas

Agree. Let's keep it manual for now to not add fields we don't use at the moment.

jsoriano · 2018-06-11T09:23:04Z

metricbeat/module/docker/network/data.go

-	event := common.MapStr{
-		mb.ModuleDataKey: common.MapStr{
-			"container": stats.Container.ToMapStr(),
+func eventMapping(r mb.ReporterV2, stats *NetStats) {


Three events would be created now:

Incoming traffic

Outgoing traffic

Legacy event

jsoriano · 2018-06-11T09:26:37Z

metricbeat/module/docker/network/_meta/data.json

+            "errors": 0,
+            "packets": 142
+        },
+        "interface": "eth0"


This event contains only inbound data, there would be another one with outbound data.

ruflin

In general I like the addition of ECS on the top level. The part I worry about is that for example winlogbeat template contains now more unused fields then before but I think it's a worthy tradeoff. Perhaps we could let each beat configure which ECS prefixes he wants to use.

Left a few comments / questions.

ruflin · 2018-06-11T10:52:22Z

libbeat/_meta/fields.ecs.yml

@@ -0,0 +1,55 @@
+- key: ecs


Agree. Let's keep it manual for now to not add fields we don't use at the moment.

ruflin · 2018-06-11T10:54:38Z

metricbeat/module/docker/network/_meta/fields.yml

@@ -33,8 +34,9 @@
            Total number of incoming packets.
    - name: out
      type: group
+      deprecated: true


We should start making use of this flag in the docs.

ruflin · 2018-06-11T10:57:08Z

metricbeat/module/docker/network/data.go

+	container.Put("runtime", "docker")
+
+	// Inbound traffic event
+	r.Event(mb.Event{


I would still expect that 1 event is sent out with all the stats inside. It seems now we send out 3 events? Why not combine inbound and outbound?

I did that initially, but then I saw that we have a network.direction field in ECS, then I though that according to ECS we'd have different events for inbound and outbound traffic. Not sure what would be the use case for this field if not used for something like this.

ruflin · 2018-06-11T10:57:25Z

metricbeat/module/docker/network/data.go

+	// Inbound traffic event
+	r.Event(mb.Event{
+		RootFields: common.MapStr{
+			"container": container,


Not sure I would mix the container to the root level into this PR.

The idea is to move the container fields to ECS structure too.

ruflin · 2018-06-11T10:57:41Z

metricbeat/module/docker/network/network.go

@@ -45,12 +44,13 @@ func New(base mb.BaseMetricSet) (mb.MetricSet, error) {
 }

 // Fetch methods creates a list of network events for each container.
-func (m *MetricSet) Fetch() ([]common.MapStr, error) {
+func (m *MetricSet) Fetch(r mb.ReporterV2) {


yay, v2 reporter.

Essential for migration to ECS 😄

ruflin · 2018-06-12T07:36:09Z

As discussed yesterday, lets move the ECS part to an other PR and continue at the moment with just adding the new fields. I still would like to see the change to v2 reporter ;-)

Collect accumulated docker network metrics and mark previous metrics as deprecated.

jsoriano · 2018-06-13T10:17:39Z

I have reverted the changes for ECS, but kept the change to v2 reporter, I opened #7301 to continue discussion on ECS. I think we should backport this for 6.3.1.

ruflin · 2018-06-13T12:23:13Z

metricbeat/module/docker/network/_meta/fields.yml

+          description: >
+            Total number of outgoing bytes.
+        - name: dropped
+          type: scaled_float


Should this be long?

Yes, all these fields should be long, I'll change it.

ruflin · 2018-06-13T12:23:24Z

metricbeat/module/docker/network/_meta/fields.yml

+          description: >
+            Total number of incoming bytes.
+        - name: dropped
+          type: scaled_float


ruflin · 2018-06-19T07:19:43Z

@jsoriano Do you want to backport it to 6.3?

Collect accumulated docker network metrics and mark previous metrics as deprecated. Fixes elastic#7240 (cherry picked from commit f0c57ea)

jsoriano · 2018-06-19T11:39:27Z

I have opened backport (#7363), but this is not a fix. In any case all related changes are already in 6.3, so I think it can make sense to merge this too.

…ork metrics (#7363) Cherry-pick of PR #7253 to 6.3 branch. Original message: Collect accumulated docker network metrics and mark previous metrics as deprecated. Fixes #7240

…er network metrics (elastic#7363) Cherry-pick of PR elastic#7253 to 6.3 branch. Original message: Collect accumulated docker network metrics and mark previous metrics as deprecated. Fixes elastic#7240

jsoriano added module review Metricbeat Metricbeat labels Jun 4, 2018

jsoriano force-pushed the docker-network branch 2 times, most recently from c58686b to 99dc422 Compare June 4, 2018 15:30

ruflin mentioned this pull request Jun 5, 2018

Revised Network Metrics elastic/ecs#2

Closed

ruflin reviewed Jun 5, 2018

View reviewed changes

jsoriano force-pushed the docker-network branch from 99dc422 to 17d6c43 Compare June 7, 2018 09:48

jsoriano added in progress Pull request is currently in progress. discuss Issue needs further discussion. and removed review labels Jun 11, 2018

jsoriano commented Jun 11, 2018

View reviewed changes

ruflin reviewed Jun 11, 2018

View reviewed changes

jsoriano force-pushed the docker-network branch from 3dcb0d8 to edb618e Compare June 12, 2018 13:55

jsoriano added review and removed discuss Issue needs further discussion. in progress Pull request is currently in progress. labels Jun 12, 2018

jsoriano force-pushed the docker-network branch from edb618e to 877c1ba Compare June 12, 2018 15:44

Metricbeat: Collect accumulated docker network metrics

5b04f29

Collect accumulated docker network metrics and mark previous metrics as deprecated.

jsoriano force-pushed the docker-network branch from 877c1ba to 5b04f29 Compare June 12, 2018 15:45

jsoriano mentioned this pull request Jun 12, 2018

Add common fields files for ECS #7301

Closed

jsoriano added the needs_backport PR is waiting to be backported to other branches. label Jun 13, 2018

ruflin reviewed Jun 13, 2018

View reviewed changes

Merge remote-tracking branch 'origin/master' into docker-network

a4a13d9

Change dropped packets count type to long

6f7303e

ruflin merged commit f0c57ea into elastic:master Jun 19, 2018

jsoriano mentioned this pull request Jun 19, 2018

Cherry-pick #7253 to 6.3: Metricbeat: Collect accumulated docker network metrics #7362

Closed

jsoriano removed the needs_backport PR is waiting to be backported to other branches. label Jun 19, 2018

jsoriano mentioned this pull request Jun 19, 2018

Cherry-pick #7253 to 6.3: Metricbeat: Collect accumulated docker network metrics #7363

Merged

jsoriano added the v6.3.1 label Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metricbeat: Collect accumulated docker network metrics #7253

Metricbeat: Collect accumulated docker network metrics #7253

jsoriano commented Jun 4, 2018 •

edited

Loading

ruflin left a comment

ruflin Jun 5, 2018

jsoriano Jun 6, 2018

ruflin Jun 6, 2018

jsoriano commented Jun 11, 2018

jsoriano Jun 11, 2018

jsoriano Jun 11, 2018 •

edited

Loading

ruflin Jun 11, 2018

jsoriano Jun 11, 2018

jsoriano Jun 11, 2018

ruflin left a comment

ruflin Jun 11, 2018

ruflin Jun 11, 2018

ruflin Jun 11, 2018

jsoriano Jun 11, 2018

ruflin Jun 11, 2018

jsoriano Jun 11, 2018

ruflin Jun 11, 2018

jsoriano Jun 11, 2018

ruflin commented Jun 12, 2018

jsoriano commented Jun 13, 2018 •

edited

Loading

ruflin Jun 13, 2018

jsoriano Jun 18, 2018

ruflin Jun 13, 2018

ruflin commented Jun 19, 2018

jsoriano commented Jun 19, 2018

Metricbeat: Collect accumulated docker network metrics #7253

Metricbeat: Collect accumulated docker network metrics #7253

Conversation

jsoriano commented Jun 4, 2018 • edited Loading

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

jsoriano commented Jun 11, 2018

Choose a reason for hiding this comment

jsoriano Jun 11, 2018 • edited Loading

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

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 commented Jun 12, 2018

jsoriano commented Jun 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruflin commented Jun 19, 2018

jsoriano commented Jun 19, 2018

jsoriano commented Jun 4, 2018 •

edited

Loading

jsoriano Jun 11, 2018 •

edited

Loading

jsoriano commented Jun 13, 2018 •

edited

Loading