Add system tests to check document fields for docker module #2971
Conversation
ruflin
force-pushed
the
mb-docker-improvements
branch
3 times, most recently
from
23e762a
to
4852f79
Compare
ruflin
added
in progress
ruflin
force-pushed
the
mb-docker-improvements
branch
4 times, most recently
from
0c881b3
to
cfd710d
Compare
ruflin
added
review
and removed
in progress
| evt = self.clean_fields(evt) | ||
| self.assert_fields_are_documented(evt) | ||
|
|
||
| def clean_fields(self, evt): |
There was a problem hiding this comment.
That's a generic name. How about remove_labels_and_ports()?
| self.assert_fields_are_documented(evt) | ||
|
|
||
| def clean_fields(self, evt): | ||
| # TODO: How to handle lables and ports? |
There was a problem hiding this comment.
We can't validate the labels so think this handling is good. Also there's typo in the comment s/lables/labels/, but I think the comment can be removed.
ports is an array of objects and I don't think we have a way to document the fields of those objects, do we? We could document the field as docker.container.ports.ip for example and ignore the fact that ports is an array. Then I think we would just need to update the python code for flatten_object to step into arrays then the assert_fields_are_documented would properly validate those fields from the objects in the array.
There was a problem hiding this comment.
I prefer to skip this for the moment as I wanted to look into how we can perhaps change the ports structure. But I agree, having something like described above could not only be useful here.
| @@ -50,6 +50,8 @@ func FetchStats(client *docker.Client) ([]DockerStat, error) { | |||
|
|
|||
| containersList := []DockerStat{} | |||
| for _, container := range containers { | |||
| // This is currently very inefficient as docker calculates the average for each request, | |||
| // means each request will take at least 1s. Getting all stats at once is implemented here: https://github.com/docker/docker/pull/25361 | |||
| containersList = append(containersList, exportContainerStats(client, &container)) | |||
There was a problem hiding this comment.
Any idea how long each request takes? Does each request block for some time period so that it can take two samples in order to calculate a delta or percentage?
There was a problem hiding this comment.
Each request takes at least 2 seconds: https://github.com/docker/docker/blob/master/cli/command/container/stats_helpers.go#L148 I'm planing to open an issue in Docker to discuss if there could be flag that raw flags are shipped. There is also moby/moby#25361 which will be useful to fetch all stats for the containers at once.
| output = self.read_output_json() | ||
| evt = output[0] | ||
|
|
||
| # TODO: How to handle lables and ports? |
ruflin
force-pushed
the
mb-docker-improvements
branch
from
cfd710d
to
156ecbb
Compare
ruflin
force-pushed
the
mb-docker-improvements
branch
3 times, most recently
from
1a3b3b6
to
9ad29d7
Compare
ruflin
force-pushed
the
mb-docker-improvements
branch
from
9ad29d7
to
5511471
Compare
Part of #2629