[metricbeat] Docker module meets metricbeat #2444
Conversation
|
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
|
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. |
| @@ -94,39 +94,39 @@ metricbeat.modules: | |||
|
|
|||
| The following metricsets are available: | |||
|
|
|||
| * <<metricbeat-metricset-system-core,core>> | |||
| * <<metricbeat-metricset-system-diskio,diskio>> | |||
There was a problem hiding this comment.
Not sure why these changes show up here as this should not have changed. I assume it is based on an old version of master.
|
To prevent future conflicts with master and review of large changes I suggest the following steps:
Then we can merge the PR as it is. There is still more work needed like some fields cleanup or adding tests. But that can be done in a second step and will make it much easier to review the PR's. Let me know in case someone has some objections? @douaejeouit Could you take care of the 4 steps above? |
|
@ruflin , Glad to read that! |
|
poke @ruflin |
|
@douaejeouit It seems like the docker client has some more dependencies which are not checked in yet. That is the reason that the travis build fails. |
douaejeouit
force-pushed
the
docker
branch
3 times, most recently
from
4c53815 to
059014d
Compare
|
Here we are .. @ruflin |
|
jenkins, test it |
|
@douaejeouit I didn't check the PR in detail yet just had a look at the files change and it seems there are still quite a few files modified which shouldn't be modified by this PR. For example related to postgresql and others. This could come from merging in an older version of master or similar. Can you rebase your changes again on top of master and then run |
|
@ruflin Ok. i'll do the needful |
douaejeouit
changed the title
|
@douaejeouit I took your branch here and run the suggested commands on top. See the outcome here: #2619 Not sure if some of the commands on your side are not running successfully? Or did you not add them to git to be commited? |
previous commits ok docker add docker module add container metricset & socket field for all the metrcsets add container metricset testing err != nil update container's metricset update docker.go update feilds TODO : add more fields add the first test unitaire : CPU metricset add other cpu calcul methods tests add mock for CPU add test for cpu metricset generator update tests for cpu metricset update test GetCpuStats add first integration test for cpu metricset complete the cpu fields complete the cpu fields update the cpu fields imports internal convention : checked. add basic integration test for the cpu metricset update code so that dockerClient instance is created only once for all the metricsets. update code so that dockerClient instance is created only once for all the metricsets & some log info text update vendors add unit tests for memory metricset mark all metricsets experimental -Merge on master .2 update vendors update vendors : remove submodule update data fields for tests
All credit for this module goes to @douaejeouit and Ingensi. See elastic#2444
All credit for this module goes to @douaejeouit and Ingensi. See #2444
|
@douaejeouit Thanks a lot for all your work on this one. We merge #2625 into master and I opened a meta issue to track the open issue / enhancements that should be made: #2629 I hope now all our future PR's will be a little bit smaller :-) Closing this one. |
Docker module wants to join you 🎉 !
Comming soon : Unit & integration tests for the rest of metricsets
@ruflin