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

Removed leaked "database" tag on redis metrics #1315

Closed
wants to merge 1 commit into from

Conversation

PierreF
Copy link
Contributor

@PierreF PierreF commented Jun 2, 2016

Required for all PRs:

  • CHANGELOG.md updated
  • Sign CLA (if not already signed)

I've found that Redis input plugin may add "database" tag on global redis stats. This is due to tags map being passed by-reference to gatherKeyspaceLine which modify it.

The result is that if you have at least one database in redis (e.g. at least one key), metric result is:

$ ./build/telegraf -config /etc/telegraf/telegraf.conf -config-directory /etc/telegraf/telegraf.d -input-filter redis -test
* Plugin: redis, Collection 1
> redis_keyspace,database=db0,host=xps-pierref,port=6379,role=master,server=172.17.0.5 avg_ttl=0i,expires=0i,keys=50000i 1464869517956355485
> redis_keyspace,database=db1,host=xps-pierref,port=6379,role=master,server=172.17.0.5 avg_ttl=0i,expires=0i,keys=1i 1464869517956396646
> redis,database=db1,host=xps-pierref,port=6379,role=master,server=172.17.0.5 clients=7i,connected_slaves=0i,ev[...] 1464869517956443591

The last line should NOT contains "database=db1" since those metrics are global.
This PR make sure that database tag is not present in redis global metrics.

@sparrc
Copy link
Contributor

sparrc commented Jun 2, 2016

This is not a safe operation. The tags map is not copied when it goes elsewhere in the telegraf system. Solution should be to copy the tags map when entering the gatherKeyspaceLine function into a new map.

@PierreF
Copy link
Contributor Author

PierreF commented Jun 2, 2016

It's indeed not copied in Telegraf. As-is is works because it's copied in InfluxDB client code. Agree it's probably not a good idea to rely on currently implementation of client to ensure a copy is done.

I'll do another PR with an explicit copy in gatherKeyspaceLine.

@PierreF PierreF closed this Jun 2, 2016
@sparrc
Copy link
Contributor

sparrc commented Jun 2, 2016

OK, thanks @PierreF!

@PierreF PierreF deleted the redis-leaked-tag branch August 4, 2018 13:23
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.

2 participants