Multiple tags

[BrandonArp]

Support using multiple template values within tags. This fixes a bug where multiple-select template fields breaks the graph.

[BrandonArp]

Also fixes the bug where the "stacked" value is cleared every time the Metrics tab is rendered.

[BrandonArp]

Rebased.

[BrandonArp]

@torkelo Can you please take a look at this real quick?

[torkelo]

Not sure how this works, is the KairosDB tag value an array? Will this not break existing use?

[torkelo]

This does not look right

[BrandonArp]

When the templateSrv.replacement is called, the value comes back as a string that looks like "{value1,value2}". Kairos accepts an array of tags. I believe that the original code wrapped single values in an array and thus needed the "flatten" call. This should not break existing functionality.

[torkelo]

this seems like a very hacky solution, KairosDB does not support glob expressions or regexes so if you want to support Multi value template variables, either KairosDB needs to support that or dont use templateSrv replace, look at the actual template variable value (it can be an array)

[BrandonArp]

You're right. Is the best way to get to the data through the templateSrv.variables? Is there a map by name somewhere? I didn't see one but might have missed it.

[BrandonArp]

Updated to be a little bit more clever about how things are done. Less hacky for sure, but much more complicated. I'm still seeing a problem where the templating isn't working like I expect. The "current" value of the variable doesn't look like it's getting updated like I expected. Is this proper behavior? Or am I thinking about this wrong?

[BrandonArp]

Disregard the previous comment. I found the proper values for templating by looking at the graphite plugin. Hopefully this is good to go now.

[jifwin]

+1

[BrandonArp]

@torkelo I updated the PR a bit to refactor it a bit and clean up the replacement in the metric name. Is this more in line with what you're looking for?

[BrandonArp]

Note: this fixes #14.

[vjkoskela]

This seems to overlap with:

#23

[BrandonArp]

#23 won't work for various reasons. I took this approach at first, but @torkelo rightly mentioned that it was hacky and brittle. Next, #23 doesn't support the "current value" for repeating panels. It also seems to not support templating of tags.

[davejhilton]

@BrandonArp -- yep. I admittedly hacked together #23 earlier today super quickly, and without having seen this pull request first. #23 was mostly just a "bare minimum" to solve the one high-priority blocker I had in my use-case, and—at first glance at least—didn't seem to regress any existing functionality.

Now that I've seen this PR, I wholeheartedly agree that this one is the far better solution. I'll close out #23 in favor of supporting this.

👍

[davejhilton]

Actually... I lied. This patch doesn't actually appear to work when adding multiple metrics via template var interpolation.

My use case (that I've attempted to address in #23) is as follows:

Create a template variable, e.g.,
$myMetrics = metrics(kairosdb.jvm)
which resolves to multiple potential values, such as:

• kairosdb.jvm.free_memory
• kairosdb.jvm.max_memory
• kairosdb.jvm.total_memory
• etc.

Then check the box for Multi Value and the box for Include All Option. Save and close the template editor.

Now create a graph panel with a datasource of KairosDB, and configure the source to use metric = $myMetrics

Now in the drop-down for $myMetrics, choose one value. With this patch, this all works thus far.

However, what doesn't work is selecting a second or third option for the myMetrics (so that 2 or 3 values are chosen). This breaks, since the template var expansion ends up creating a kairosdb query that looks like this:

{
  "metrics": [
    {
      "name": [
        "kairosdb.jvm.free_memory",
        "kairosdb.jvm.max_memory"
      ]
    }
  ],
  ...
}

when it needs to create a query like this instead:

{
  "metrics": [
    {
      "name": "kairosdb.jvm.free_memory"
    },
    {
      "name": "kairosdb.jvm.max_memory"
    }
  ],
  ...
}

(and then obviously it needs to handle the response properly etc).

My code in #23 is currently pretty hacky... and doesn't even attempt to address the tags stuff, but the multi-valued metrics do work in the cases I've tried. So rather than closing #23 like I mentioned above, I'll probably attempt to clean it up so that multiple metrics values work while also supporting the "current value" functionality. I'll probably leave the tags fixes for the scope of this patch though (#18) rather than trying to tackle that as well. The other option would be for me to fork this branch and try applying my fix on top of this one. Not sure which is a better approach but I'm open to suggestions. Additionally, I'm open to any kind of criticism / alternate implementation ideas on what #23 is doing that anyone else might have. This is all pretty new to me so I don't even pretend to know what I'm missing!

Thanks!

[BrandonArp]

@davejhilton You're absolutely right. Great write-up on what was wrong. I've updated the PR to reflect the fix. Your PR was a great help. Regardless of how they get merged, I'm quite convinced that we should have some way to test changes like these. The edge cases alone are absolutely crazy!

[davejhilton]

@BrandonArp Nice! The updated patch works perfectly now for everything I tested with regards to tags and metrics (even the "All" case for tags, which wasn't working before, but I had neglected to mention previously). I'm closing #23 now for real, since this covers it far better.

👍

[gitvegas]

How can I get and install this patch, please suggest, appreciate your help, cheers