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

Improve docker input plugin #754

Closed
wants to merge 1 commit into from

Conversation

titilambert
Copy link
Contributor

Hello !
This commit adds docker info data in returned metrics and improve test coverage to more than 70%

@sparrc
Copy link
Contributor

sparrc commented Mar 1, 2016

thanks @titilambert, can you update the README with the new stats?

@titilambert
Copy link
Contributor Author

Done !

@sparrc
Copy link
Contributor

sparrc commented Mar 2, 2016

the way the metrics are formatted seems a little inconsistent to me, what about like this?

- docker
    - n_used_file_descriptors
    - n_cpus
    - n_containers
    - n_images
    - n_goroutines
    - n_listener_events
    - memory_total
    - pool_blocksize
- docker_data
    - available
    - total
    - used
- docker_metadata
    - available
    - total
    - used

@titilambert titilambert force-pushed the docker branch 2 times, most recently from 7da18f4 to 6ec9d91 Compare March 2, 2016 13:54
@titilambert
Copy link
Contributor Author

@sparrc you're right ! Changes done !

@tripledes
Copy link

@titilambert hey, looking at the diff just spotted a typo:

docker (memeory_total)

Nice job btw!

@titilambert
Copy link
Contributor Author

@tripledes Thanks ! Fixed !

}

// Add metrics
acc.AddFields("docker",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change these to a single call to AddFields():

fields := map[string]interface{}{
  "n_cpus": ...
  "n_containers": ...
  ...
}
acc.AddFields("docker", fields, ...)

@titilambert
Copy link
Contributor Author

Done ! I didn't merge memory_total because there is unit tag with.

@sparrc sparrc closed this in 1c76d5d Mar 7, 2016
@titilambert titilambert deleted the docker branch March 7, 2016 19:58
geodimm pushed a commit to miketonks/telegraf that referenced this pull request Mar 10, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants