Fixes for InfluxDB queries

[pdf]

• Fix a typo to allow building
• Add support for backtick-quoted strings, because double-quotes are really a necessity for InfluxDB queries

[pdf]

Refs #618

[pdf]

Hrm, any idea how to fix travis to install deps on this branch?

[captncraig]

the influx files need to be vendored to the _third_party directory with github.com/mjibson/party

[pdf]

The current tests appear to require a real connection to a DB and some arbitrary data, so that's not going to fly. Other sources don't include any tests, are we better to just delete the integration tests?

[maddyblue]

Backticks won't work because the conf file already uses backticks. However I added single quotes, so you can use those:

alert a {
  crit = influx('select * from "db"."retention"."measurement"', ...)
}

Could you un-party the code for now (remove the new _third_party stuff and point the imports back to their normal place)? We'll do that in another PR so we can actually see this and post comments on github. It's currently too big to render it all. Don't worry about the travis stuff.

The tests were just there so I could type go test and get something to run. They will not be committed as is since they require a real database. They'll either be removed or mocked.

[pdf]

Backticks won't work because the conf file already uses backticks.

I don't follow, do backticks have some special conflicting use in the configs?

However I added single quotes, so you can use those

Single quotes don't work from the expression UI for me - I get expr: invalid syntax, I haven't traced this yet. I see strconv.Unquote() used in a couple of places - that expects a character literal if it encounters single-quoted input.

EDIT: I've rolled back the party commit.

[pdf]

So apparently (influxdata/influxdb#1757) identifiers need to be double-quoted, and string-literals need to be single-quoted, so it looks like we'd need escaping if backticks are out of the question.

[pdf]

It also looks like we can't currently do GROUP BY time() due to splitting out the time range as parameters. The InfluxDB parser barfs when the group is included but there're no time conditions, and the Bosun code needs to parse the query before it can append the time conditions. This is another blocker. Thoughts on the best approach?

[maddyblue]

I don't follow, do backticks have some special conflicting use in the configs?

Yes. Multi-line strings are created using backticks, and there's no escaping, so you can't have a backtick in a backtick-delimited string.

identifiers need to be double-quoted, and string-literals need to be single-quoted

Ugh. I didn't know that. I thought double quotes were all it used. This means Bosun will have to add in escape characters to either its conf or expr parsing. This needs a bit more discussion before a decision is made about where to do the escaping.

InfluxDB parser barfs when the group is included

Hmm. I'll look into this more. No obvious solutions come to mind.

[nathanielc]

@mjibson Any thoughts on the direction bosun should take with this?

Looking to get InfluxDB support across the line. I have been starting here https://github.com/bosun-monitor/bosun/compare/influxdb-query

[maddyblue]

I'm no longer involved in Bosun, but I assume it's where it was a few months ago. If someone is willing to update the existing influx PR and fix the issues listed here and will maintain it going forward, the team may merge it.

[kylebrandt]

@nathanielc I think it would be best just to go with triple quoted strings as another format of literal strings since we already use backticks. The parser will need to be updated for that though.

[nathanielc]

@kylebrandt ok thanks, I'll take a crack at it.

[Dieterbe]

I'm no longer involved in Bosun

miss you already @mjibson ! hope you're having a good time with the new gig.

[nathanielc]

@kylebrandt See #1289 This should address the issues that have been discussed.

[gbrayut]

Closing this issue as InfluxDB support was added in #1291