-
Notifications
You must be signed in to change notification settings - Fork 491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd/bosun: InfluxDB query support #1291
cmd/bosun: InfluxDB query support #1291
Conversation
Thanks for squashing the commits. We usually prefer things get squashed before merging, and I see you already used the cmd/bosun: prefix. I'll take a look today and see if I can get it running. Also for future reference you can force push the squashed commits to an existing branch instead of opening a new PR. A new PR is only needed if you are deviating from the original goal or want to reset the discussion. Using the existing PR can help keep all the comments in one place. I'll let you know how much progress I get on this later today. |
First off, some notes from how I did my testing:
Using the above I now have: a quick test on the bosun expression page shows the following returns the expected data: I will continue testing and let you know if I find any issues. |
Triple quotes give an error: I patched that in parser.go in my branch at https://github.com/giganteous/bosun/tree/squashed_influxdb-query |
This doesn't need to be addressed in this PR but we may want to improve the logging of which backends are being used and what their settings are (tsdbHost, graphiteHost, influxHost, logstashElasticHosts) We should probably make a docker image for testing. I'm happy to file an issue for tracking and work on this next week. I'll chat with Craig about this and see when we can merge it. |
docs: Add docs for influx function
@giganteous thanks for the patch. I have applied it and updated this PR |
The updated PR LGTM. The following expressions work on my local instance:
I'll merge this and create issues for tracking some followup items (how to guide, docker image, etc). If you find any problems please create an Issue/PR using the influxdb label. Thanks @nathanielc and @giganteous! |
cmd/bosun: InfluxDB query support
I am getting below error while trying to start bosun. sudo ./bosun -c conf/bosun.conf goroutine 1 [running]: goroutine 17 [syscall, locked to thread]: goroutine 5 [runnable]: goroutine 6 [runnable]: goroutine 7 [runnable]: goroutine 8 [runnable]: goroutine 11 [runnable]: goroutine 14 [runnable]: goroutine 16 [runnable]: goroutine 18 [runnable]: goroutine 19 [runnable]: goroutine 20 [runnable]: My config file looks like influxHost = 54.254.204.231:8086 template cpu {
} notification default { alert influx.os.high.cpu { What is wrong in the configuration? |
This bug is fixed if you compile from master if I'm not mistaken. |
Here is an updated PR from the original 'influxdb-query' branch. It adds support for an influx function that can perform a query against InfluxDB and return the results as a seriesSet. It also adds support for triple quoted strings in expressions so that InfluxDB queries make full use of single and double quotes.
Other small changes where made from the original PR:
Manipulating the InfluxQL AST instead of string manipulations
Removed the format parameter since it required that the expression double list desired tags. Seems like the original reason for having it was to get around a bug in InfluxDB and is no longer necessary. See influxdata/influxdb#3059
Now squashed with all changes.