expvarmon icon indicating copy to clipboard operation
expvarmon copied to clipboard

Feature - duration string var type

Open cklenetsky opened this issue 6 years ago • 3 comments

I have a number of stats that export as a duration string, such as:

"uptime": "32m48.4137762s" or "averagerequesttime": "67.6394ms"

These don't generate spark lines, nor are their max really valid. How about an additional var type, durationstr, that parses the duration back into an int64 for comparison/sparkline, but still displays with the duration string representation for labeling?

cklenetsky avatar Dec 12 '18 18:12 cklenetsky

Hi @cklenetsky, thanks for the issue and PR! It's a really interesting case, and I assume you don't have any access to the metrics producing code, right? I like the PR, it's clean and simple, however on thing bugs me – this sort of metrics is just the wrong way of reporting durations, and should be fixed anyway. Forcing all the metric consumers to parse the string representation of duration seems sub-optimal, and introducing a new variable formatter as a workaround seems a bit too much.

The main consideration, of course, is to keep a number of options and modifiers in expvarmon as small as possible. So I would try not to add new formatters unless it's the really common case and heavily requested.

I see two solutions/workarounds for this problem:

  1. Update metrics producing code (it seems to be the most right solution, yet not always possible)
  2. Use the forked version of expvarmon. It doesn't change often (maybe on small PR per year, haha), so you will not lose anything in using forked version, that's just ok.

What do you think?

divan avatar Dec 17 '18 14:12 divan

Hi Ivan,

I actually do have the ability to change the metric-producing code, but it's also dependent upon the product manager/customer's wishes. For the moment, the duration string was a very easy way to display the info in an immediately-readable fashion, which is why I spit out the info that way. As you point out, it's not the most common/necessary way. It's just how our app is currently doing it, and I wasn't sure if anyone else is as well. I may, actually, spit out the stat in both forms -- as the duration string and as an int64.

Either way, I can certainly keep the change in a fork. For now I'm the only one using it to watch over running instances of our app. It's likely that the customer is going to integrate the stats-checking into their own monitoring system. But if you think there's anyone else that may want it, the PR is there whenever you need it.

cklenetsky avatar Dec 17 '18 19:12 cklenetsky

@cklenetsky, perfect! Yes, I think keeping both versions of metrics is a way to go in your case.

divan avatar Dec 18 '18 09:12 divan