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

Metricbeat docker diskio module bug #5568

Closed
kwojcicki opened this issue Nov 13, 2017 · 5 comments
Closed

Metricbeat docker diskio module bug #5568

kwojcicki opened this issue Nov 13, 2017 · 5 comments
Labels

Comments

@kwojcicki
Copy link
Contributor

kwojcicki commented Nov 13, 2017

  • Version:
    Any with docker diskio module
  • Operating System:
    Any
  • Steps to Reproduce:
    Start 2+ containers check the docker diskio statistics pushed. The stats will always be 0 due to a bug in the code.

There is a bug in /metricbeat/module/docker/diskio/help.go that resets the map of containers if a old stats for a container arent found.

func (io *BLkioService) getBlkioStats(myRawStat *docker.Stat) BlkioStats {
......
       if exist {
		myBlkioStats.reads = io.getReadPs(&oldBlkioStats, &newBlkioStats)
		myBlkioStats.writes = io.getWritePs(&oldBlkioStats, &newBlkioStats)
		myBlkioStats.totals = io.getTotalPs(&oldBlkioStats, &newBlkioStats)
	} else {
		io.BlkioSTatsPerContainer = make(map[string]BlkioRaw)
	}

If there are 2+ containers then the map will be reset over and over again ie:
getBlkioStats gets called for first container
old stats for first container dont exist, map reset, container1 placed into map
getBlkioStats gets called for second container
old stats for second container dont exist, map resets, container2 placed into map

rinse and repeat. I have a code fix + tests not sure if I should push into master or into a lower release and cherry pick into higher releases as well.

@kwojcicki kwojcicki changed the title Metricbeat Bug Metricbeat docker diskio module bug Nov 13, 2017
@andrewkroh
Copy link
Member

@kwojcicki Thanks for reporting. Please open a PR for this. I recommend starting with a PR for master and we can backport it from there.

@kwojcicki
Copy link
Contributor Author

Sounds good. I was having some issues pushing to the elastic beats, git was complaining about some 403 error? Probably not the best place to ask this but if you have had any experience with dealing with that issue would be helpful 😄

@andrewkroh
Copy link
Member

You'll need to create a fork, push the changes to your fork, then open a PR from your fork to this repo.

@joelstewart
Copy link

will this be merged into a 5.x release?

@kwojcicki
Copy link
Contributor Author

Once PR 5582 closes I can look into making a PR for 5.5 or 5.6 shouldnt require any extra effort to back port.

kwojcicki added a commit to kwojcicki/beats that referenced this issue Nov 23, 2017
* Fixed docker diskio bug due to reseting of map.

Fixes elastic#5568
kwojcicki added a commit to kwojcicki/beats that referenced this issue Nov 23, 2017
* Fixed docker diskio bug due to reseting of map.

Fixes elastic#5568
ruflin pushed a commit that referenced this issue Nov 27, 2017
* Fixed docker diskio bug due to reseting of map.

Fixes #5568
ruflin pushed a commit that referenced this issue Nov 27, 2017
* Fixed docker diskio bug due to reseting of map.

Fixes #5568
leweafan pushed a commit to leweafan/beats that referenced this issue Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants