Metricbeat: Collect accumulated docker network metrics#7253
Metricbeat: Collect accumulated docker network metrics#7253ruflin merged 3 commits intoelastic:masterfrom
Conversation
c58686b to
99dc422
Compare
There was a problem hiding this comment.
To follow ECS, should it be on the root level? Meaning having it under network.inbound.bytes ?
There was a problem hiding this comment.
Umm, you are right, I was misinterpreting ECS for this case, would it be ok to have this without the docker preffix?
There was a problem hiding this comment.
If the content of the field matches ECS I would say yes.
|
@ruflin I have done the changes for more proper ECS, this would probably require its own PR, but lets discuss all together before. |
libbeat/_meta/fields.ecs-ext.yml
Outdated
There was a problem hiding this comment.
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.
libbeat/_meta/fields.ecs.yml
Outdated
There was a problem hiding this comment.
This file should probably be automatically generated from https://github.com/elastic/ecs/tree/master/schemas
There was a problem hiding this comment.
Agree. Let's keep it manual for now to not add fields we don't use at the moment.
There was a problem hiding this comment.
Three events would be created now:
- Incoming traffic
- Outgoing traffic
- Legacy event
There was a problem hiding this comment.
This event contains only inbound data, there would be another one with outbound data.
ruflin
left a comment
There was a problem hiding this comment.
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.
libbeat/_meta/fields.ecs.yml
Outdated
There was a problem hiding this comment.
Agree. Let's keep it manual for now to not add fields we don't use at the moment.
There was a problem hiding this comment.
We should start making use of this flag in the docs.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Not sure I would mix the container to the root level into this PR.
There was a problem hiding this comment.
The idea is to move the container fields to ECS structure too.
There was a problem hiding this comment.
Essential for migration to ECS 😄
|
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.
|
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. |
| description: > | ||
| Total number of outgoing bytes. | ||
| - name: dropped | ||
| type: scaled_float |
There was a problem hiding this comment.
Yes, all these fields should be long, I'll change it.
| description: > | ||
| Total number of incoming bytes. | ||
| - name: dropped | ||
| type: scaled_float |
|
@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)
|
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. |
…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
Collect accumulated docker network metrics and mark previous metrics as deprecated.
Fixes #7240