Feature/inline tags for all #55
Conversation
|
OK, just discovered those automated travis UT checks. Nevertheless, I'ms struggling to debug my Prometheus UT, due to the aforementioned reasons 😧 |
| /** | ||
| * InlineTaggableGaugeableCollector interface. | ||
| */ | ||
| interface InlineTaggableGaugeableCollector |
There was a problem hiding this comment.
You can not remove an interface, that's a BC break.
I did not look at the code for now, but If you have to do it, you have to depreciate it and provide an upgrade path.
But honestly, I prefer to not remove it to not break the BC.
This PR was squashed before being merged into the 2.x-dev branch (closes beberlei#56). Discussion ---------- Added in memory collector Hi. I needed to measure processes' metrics and retrieve them in same request/run and hook it up with existing storing procedures. It will be useful to stick to same interface later when we will move that outside to graphite or something. I think it may be useful for others too in some cases. Commits ------- 466ee27 Added in memory collector
This PR was merged into the 2.x-dev branch. Discussion ---------- Fixed CS Commits ------- 0fb6d89 Fixed CS
|
@lyrixx: I would like to pick up this feature for Prometheus. As Prometheus will switch from |
|
@buffcode : you do not need to remove the setTags(), it could still be used for global tags, shared by all metrics sent with the Prometheus instance. I sadly didn't quite got the time to work on this branch lately, and would gladly accept someone taking ownership or just helping, specifically with the prometheus UT. I can't debug them on my M$ Windows station, as the prometheus lib won't download via composer on this OS... |
👍 This PR needs a rebase BTW. |
…o feature/inline-tags-for-all # Conflicts: # src/Beberlei/Metrics/Collector/InlineTaggableGaugeableCollector.php
|
🙌 OK, I merged (rebase wasn't happy, probably due to the deleted InlineTaggableGaugeableCollector.php) and finally managed to debug the failing Prometheus UTs. I added back the Indeed:
@lyrixx : I would like some guidelines regarding how to mark the |
|
OK, I made the This way, we should not have any BC break. @lyrixx, are you OK with this approach ? |
|
Any news concerning this PR ? |
|
Ouspy, accidental close |
PR to answer enhancement reported in issue #53
Implementation details:
Some stuff to look for before merging:
Enjoy!