Support requesting only active tags from influxdb in a time range #7919

mkutsevol · 2017-03-23T13:27:00Z

It adds two things to templating.

If the query starts with a select, then it takes the second column from the output (skips the time column)
$timeFilter is processed as in annotation queries.

Example usage:

In template query input something like this:
SELECT pod_name,last(value) FROM uptime WHERE $timeFilter AND namespace_name = '$namespace' GROUP BY pod_name

The output from this query is time, pod_name lines, and pod_name is being taken.
The query output will be limited to only active pod_names in the time range!

Works like a charm for me :D

CLAassistant · 2017-03-23T13:28:01Z

All committers have signed the CLA.

mkutsevol · 2017-03-23T13:33:40Z

Also #5013

kubernetes-retired/heapster#1179

torkelo · 2017-03-28T06:24:35Z

public/app/plugins/datasource/influxdb/response_parser.ts

+      var part = query.substr(query.indexOf(';;'));
+
+      arbitraryColumn = part.split(';').length - 2;
+      console.log(arbitraryColumn);


Can you describe what this part does? and please remove console log line :)

It counts the number of ';', starting from the two consecutive ';;'

and represents it as an index (the -2 part)

torkelo · 2017-03-28T06:29:58Z

I think the use of ; as column selector needs to change, creative solution but not great :)

mkutsevol · 2017-03-28T06:39:53Z

How did the log() get through :D
The other way is to modify the interface, 'cos besides ; I don't think we can use anything else

mkutsevol · 2017-03-28T06:47:21Z

@torkelo , thank you for the review, pushed an update

mkutsevol · 2017-03-28T07:09:23Z

Also there is a way to keep the interface as is and use a regexp.
I can join all fields in influxdb return with some string (like ___ or $$$ or any other string with a low probability of encountering in the wild) and then use regexp to extract the needed part. @torkelo , what's your opinion?

torkelo · 2017-03-28T08:41:35Z

I can join all fields in influxdb return with some string (like ___ or $$$ or any other string with a low probability of encountering in the wild) and then use regexp to extract the needed part. @torkelo , what's your opinion?

Not sure what your trying to solve. Is the problem figuring out which field (column) to use in the InfluxDB response? Can you not write the query so that it only returns a single column? (beside time)

mkutsevol · 2017-03-28T10:25:08Z

@torkelo, you can see the precise description of the problem and the workarounds taken (not solved) in heapser issue kubernetes-retired/heapster#1179

In influxdb I can't change the position of the time field, and I cant make it output my_tag, time. It does time, my_tag. To output two columns only, you need really latest influx with subquery support.

The problem:
In short - imagine a situation, when you have a high rotation of tags. The tags/names/hostnames whatever are short lived. When I tried vanilla grafana on my influxdb after a week of data collection it tried to render 10k tags in one namespace. 9900 of them being dead and not being interesting to me at all. I wanted stats for the past 30mins from the last rollout.

torkelo · 2017-03-28T11:44:30Z

I understand the bigger problem (query for only active series/tags). But not the need to use number of ; to know which column to use

mkutsevol · 2017-03-28T13:04:48Z

@torkelo, I'm open to any proposal you've got.
How this should be implemented, share your idea!

torkelo · 2017-03-28T13:13:53Z

is not always the second column? (or first non time column) ?

mkutsevol · 2017-03-28T13:37:54Z

No, it is not,
It is up to the user to construct the query. If SHOW metadata query is used then it will output one column. If SELECT is used when whatever the user wrote is in the result.

Sure, parsing for select query and (always) taking the second column will work, but is it better than a special query syntax, which imposes no constraints on the user which writes the query?

torkelo · 2017-03-28T14:16:28Z

but is it better than a special query syntax, which imposes no constraints on the user which writes the query?

yes, by miles compared to using a the number of semi colons at the end of the statement :)

mkutsevol · 2017-03-28T15:04:41Z

@torkelo ok, not a problem at all.

mkutsevol · 2017-03-28T15:14:34Z

@torkelo, I've pushed a variant w/o semicolumns :D
I don't squash it while you are reviewing it, will do as it will be ready for merge.

nocamin · 2017-04-25T08:54:58Z

I'm unable to get the second column values in templating, just got the time column in Preview of values (shows max 20) option.

Can you write a simple query for me to get the second column output in templating.

OR How to get the only second column output ?

nocamin · 2017-04-25T09:00:44Z

All changes done as per given in two Commits. But still getting time column only as per screenshot.

mkutsevol · 2017-04-27T10:09:49Z

@nocamin
your query is correct. If you applied this PR it should just work.
There is no activity in it after my last posts, so I assume the authors are not interested in this use case.

torkelo · 2017-04-28T08:36:46Z

this is very close to mergable.

accessing this.timeSrv.time is not good, should call this.timeSrv.timeRange(),
missing tests change to influxdb response_parser

yinhua-dai · 2017-05-02T02:20:07Z

@torkelo , when I can get a formal package for this? Thank you.

daniellee · 2017-05-02T08:58:43Z

@yinhua-dai this is not merged yet as it still needs changes. So hard to say which Grafana release it will be included in.

mkutsevol · 2017-05-02T13:42:46Z

@torkelo, thanks for feedback, will update soon.

Darkmyr · 2017-07-10T16:10:38Z

@mkutsevol any updates? Would love to see this in grafana.

Thanks,
Darkmyr

mkutsevol · 2017-07-10T16:40:46Z

Switched jobs ~ a couple of month ago, was running out of time completely, moving to a new city, etc.
I remember about my open PRs on github, will try to do my best to address the issues and get it merged.

mkutsevol · 2017-07-10T18:19:58Z

@torkelo, hi!
can I get a review here?
Thanks!

Darkmyr · 2017-07-10T18:59:46Z

@mkutsevol Thank you! I appreciate you taking the time.

Darkmyr

mkutsevol · 2017-07-10T19:02:57Z

@Darkmyr, it's my pleasure :D

elyzion · 2017-10-20T09:00:31Z

I really need this for my AWS environments as well. I'm using immutable deployments for my Elastic Beanstalk and this results in a horde of old instances showing up when I try and filter by host tag.

mkutsevol · 2017-10-23T13:30:39Z

Seems like removing one on my email addresses from GH account revoked CLA here, will push again.
@torkelo is anything else needed to get it merged?
Thanks!

* Added test case * Switched to timeSvr.timeRange()

daniellee · 2017-10-31T11:37:14Z

Would it be possible to do this the same way we do for the Postgres and MySQL data sources with an alias for the field name. Something like:

SELECT pod_name as "__text", last(value) FROM uptime WHERE $timeFilter AND namespace_name = '$namespace' GROUP BY pod_name

and maybe could support both text and value like this:

SELECT pod_name as "__text", pod_template_hash as "__value", last(value) FROM uptime WHERE $timeFilter AND namespace_name = '$namespace' GROUP BY pod_name

mkutsevol · 2017-10-31T21:04:50Z

Unfortunately, no.
It's not an SQL db, they just try not to invent a completely new syntax.

daniellee · 2017-10-31T21:28:42Z

Don’t understand. InfluxDB has support for field alias. So why is not possible?

mkutsevol · 2017-10-31T21:38:14Z

Ah, ok. I haven't checked influx. You may be right. But I don't have any more will to change this PR. At least in a dramatic way. And I don't see a clear benefit in that approach with aliases.

daniellee · 2017-11-01T08:27:28Z

@mkutsevol I'm sorry we have been so slow with feedback and that you don't want to work on the PR anymore. We felt that this solution was a bit clunky but we discovered a better way when working on mysql/postgres and I only just now made the connection that this PR is really similar.

Is it OK if I take over the PR and finish it then?

The benefit with aliases is that:

you do not have to hard code that the text value is the second value in the list of columns. (this code -> addUnique(res, value[1] || value[0]);)
You can have a template variable with a text value (that is a user friendly value) and a value that is not so user friendly but is used in the query. E.g. text: server name value: ip address
It is not a big change to your PR. And this type of parsing has already been done in the mysql and postgres response parsers if a reference point is needed.

mkutsevol · 2017-11-01T09:47:47Z

@daniellee, yes, sure, take it over!
Thanks!

torkelo · 2018-05-07T13:16:05Z

closing this PR as it targets an old branch, and the author has abandoned it. Maybe this PR can solve this, #11846

torkelo added type/feature-request pr/early-feedback WIP state but looking for early feedback labels Mar 24, 2017

daniellee self-assigned this Mar 27, 2017

torkelo reviewed Mar 28, 2017

View reviewed changes

torkelo added pr/changes-needed pr/needs-unit-tests Unit tests are required before merge labels Mar 28, 2017

mkutsevol force-pushed the feature/templating_influx_time_filter branch from efb5c81 to 1db369a Compare March 28, 2017 06:46

mkutsevol force-pushed the feature/templating_influx_time_filter branch 2 times, most recently from 25c8267 to ffb157b Compare July 10, 2017 18:11

mkutsevol added 3 commits October 23, 2017 16:34

Allow special syntax to get value from arbitrary column in influxdb

8eb119b

Switch to determining select query and always using the second column

e266f5d

Address the CR comments

c420bdc

* Added test case * Switched to timeSvr.timeRange()

mkutsevol force-pushed the feature/templating_influx_time_filter branch from ffb157b to c420bdc Compare October 23, 2017 13:35

torkelo closed this May 7, 2018

ying-jeanne added the pr/external This PR is from external contributor label Apr 29, 2021

Support requesting only active tags from influxdb in a time range #7919

Support requesting only active tags from influxdb in a time range #7919

Conversation

mkutsevol commented Mar 23, 2017 • edited

CLAassistant commented Mar 23, 2017 • edited

mkutsevol commented Mar 23, 2017 • edited

torkelo Mar 28, 2017

Choose a reason for hiding this comment

mkutsevol Mar 28, 2017 • edited

Choose a reason for hiding this comment

mkutsevol Mar 28, 2017

Choose a reason for hiding this comment

torkelo commented Mar 28, 2017

mkutsevol commented Mar 28, 2017 • edited

mkutsevol commented Mar 28, 2017 • edited

mkutsevol commented Mar 28, 2017 • edited

torkelo commented Mar 28, 2017

mkutsevol commented Mar 28, 2017

torkelo commented Mar 28, 2017

mkutsevol commented Mar 28, 2017

torkelo commented Mar 28, 2017

mkutsevol commented Mar 28, 2017

torkelo commented Mar 28, 2017

mkutsevol commented Mar 28, 2017

mkutsevol commented Mar 28, 2017

nocamin commented Apr 25, 2017

nocamin commented Apr 25, 2017

mkutsevol commented Apr 27, 2017

torkelo commented Apr 28, 2017

yinhua-dai commented May 2, 2017

daniellee commented May 2, 2017

mkutsevol commented May 2, 2017

Darkmyr commented Jul 10, 2017

mkutsevol commented Jul 10, 2017

mkutsevol commented Jul 10, 2017

Darkmyr commented Jul 10, 2017

mkutsevol commented Jul 10, 2017

elyzion commented Oct 20, 2017

mkutsevol commented Oct 23, 2017

daniellee commented Oct 31, 2017 • edited

mkutsevol commented Oct 31, 2017

daniellee commented Oct 31, 2017

mkutsevol commented Oct 31, 2017

daniellee commented Nov 1, 2017

mkutsevol commented Nov 1, 2017

torkelo commented May 7, 2018

mkutsevol commented Mar 23, 2017 •

edited

CLAassistant commented Mar 23, 2017 •

edited

mkutsevol commented Mar 23, 2017 •

edited

mkutsevol Mar 28, 2017 •

edited

mkutsevol commented Mar 28, 2017 •

edited

mkutsevol commented Mar 28, 2017 •

edited

mkutsevol commented Mar 28, 2017 •

edited

daniellee commented Oct 31, 2017 •

edited