Add the datasource of RDBMS (PostgreSQL and MySQL) [backend part] #5364

Open
wants to merge 3 commits into
from

Projects

None yet
@anzai
anzai commented Jun 15, 2016

This is the changes of the backend part of #5168.
The frontend part was separated, and will be distributed as an external plugin here:
https://github.com/sraoss/grafana-sqldb-datasource .

What each paerts of this datasource are:

  1. frontend: send SELECT statements via HTTP POST
  2. backend: connect to RDBMS and execute SELECT statements with xorm
  3. backend: send the results to the frontend
  4. frontend: arrange the result values for desired data types (collected numeric value, table names as text, and so on)

For some helps to test this, I created the Grafana binary including this datasource plugin in data/plugins.
https://github.com/sraoss/grafana-sqldb-datasource/releases/download/V0.1/grafana-sqldb_20160615.tar.gz

@anzai anzai Add the datasource of RDBMS (PostgreSQL and MySQL)
6586cc4
@bergquist
Contributor

Great! We will look into this PR but we are really swamped with other work at the moment so it might take some time before we get to it. I think this can be awesome! :)

@torkelo
Member
torkelo commented Jun 15, 2016

yes, don't be disheartened by the delay in PR review/feedback and merge. We will have more time to focus on this after 3.1 is released and alerting merged to master (after 3.1 is released).

@neshwill
neshwill commented Jun 29, 2016 edited

Hi , i try to use this patch , and have an error after i replace files in my grafana directories,

template: index:65:16: executing "index" at <.BuildVersion>: BuildVersion is not a field of struct type *dtos.IndexViewData

and

2016/06/29 16:32:20 [I] Completed 10.19.0.26 admin "GET / HTTP/1.1" 500 Internal Server Error 124 bytes in 2437us 2016/06/29 16:32:32 [I] Completed 10.19.0.26 admin "GET / HTTP/1.1" 500 Internal Server Error 124 bytes in 1533us

Maybe i do something wrong ?

@anzai
anzai commented Jun 30, 2016

@neshwill Thank you for trying.
I've never seen such message. Which files did you replace?

@anzai anzai Fix wrong overwrite of commit:711992c8e2e811abd450cb665008bb0a1b3e29a9
bdf68b2
@neshwill

Hi , Thank you for so fast reply. What i try to do, is

a. Install rpm -i grafana-3.0.4-1464167696.x86_64.rpm b. when i download https://github.com/sraoss/grafana-sqldb-datasource/releases/download/V0.1/grafana-sqldb_20160615.tar.gz c. Extract grafana-sqldb_20160615.tar.gz d. When replace all folder from grafana-3.0.4-1464167696.x86_64.rpm , i mean /usr/share/grafana on folders from grafana-sqldb_20160615.tar.gz , and bin/ on bin from grafana-sqldb_20160615.tar.gz . e. When i am start grafana , it's starting , and i have error , i mention before .

Maybe i do some wrong ?

@anzai
anzai commented Jun 30, 2016

@neshwill I'm not sure if your way could be ok or not. I expect as follows:

a. Extract grafana-sqldb_xxx.tar.gz in directory as you like
b. Start Grafana with executing bin/grafana-server

@neshwill

Thank you very much! Work nice !

@nitram4
nitram4 commented Jul 5, 2016

Sorry, maybe stupid question but this will not in any way work with Oracle DB?

@z3ugma
z3ugma commented Jul 7, 2016

Hey @anzai, this patch was working quite well for me yesterday but when I stopped and restarted the grafana server I got the same error as above.

In browser: template: index:65:16: executing "index" at <.BuildVersion>: BuildVersion is not a field of struct type *dtos.IndexViewData

In CLI: 2016/07/07 08:42:40 [I] Completed <IP ADDRESS> - "GET /login HTTP/1.1" 500 Internal Server Error 124 bytes in 3042us

I downloaded https://github.com/sraoss/grafana-sqldb-datasource/releases/download/V0.1/grafana-sqldb_20160615.tar.gz and extracted it, no messing around with anything.

@theangryangel
Contributor

@nitram4 it would require an Oracle driver to be added to this PR (I'm planning on testing with SQL server myself, when I have a moment), and then the frontend plugin updated to generate the correct SQL.

@nitram4
nitram4 commented Jul 9, 2016

@theangryangel I dont have knowledge hoe to do it, but if you are successfull with SQL server and provide basic instructions I can try it :-)

@anzai
anzai commented Jul 11, 2016

@z3ugma Thank you for testing.
How did you restart Grafana at that time? If you have installed stable Grafana also by RPM and restarted by its init script, such message might be shown. The backend of stable version cannot deal with the frontend of sqldb plugin.

@anzai
anzai commented Jul 11, 2016

@theangryangel @nitram4 It'll be certainly better to deal with more kinds of RDBMSs, but I think we need to discuss if another approach to deal with RDBMS would be better than this one or not. We would better to discuss the kinds of RDBMSs after fixing the architecture of the backend?

@torkelo
Member
torkelo commented Jul 11, 2016 edited

Trying to test this out, but getting this error for any query I try:

Response:

Data error: Error 1055: Expression #1 of SELECT list is not in GROUP BY clause and contains 
nonaggregated column 'grafana.Metrics.timestamp' which is not functionally dependent on columns in 
GROUP BY clause; this is incompatible with sql_mode=only_full_group_by, 
Query: SELECT (UNIX_TIMESTAMP(timestamp) DIV 1) * 1 * 1000 AS time_msec, sum(Value) FROM grafana.Metrics WHERE timestamp \u003e DATE_SUB(CURRENT_TIMESTAMP(), INTERVAL 15 MINUTE) GROUP BY (UNIX_TIMESTAMP(timestamp) DIV 1) * 1 ORDER BY (UNIX_TIMESTAMP(timestamp) DIV 1) * 1"
@anzai
anzai commented Jul 12, 2016

@torkelo Sorry, I forgot the case of "sql_mode = only_full_group_by" in MySQL. I fixed the bug of the frontend.

@ptMuta
Contributor
ptMuta commented Jul 14, 2016

Getting this to 4.0 would be fantastic. If there's anything I can do to help, drop me a line

@torkelo
Member
torkelo commented Jul 14, 2016

I definitively think this can be merged to master soon. I tested it a few days ago.

The main thing is the hard coding of the plugin type,
https://github.com/anzai/grafana/blob/bdf68b21836050b6abced624f36f7acff58cef88/pkg/models/datasource.go#L19

The SQL data proxy should be more generic so that multiple frontend plugins can use the SQL data proxy path.

@mtanda
Contributor
mtanda commented Jul 22, 2016

I may found a problem in this PR.
The backend proxy need to check whitelist, I think.

https://github.com/anzai/grafana/blob/bdf68b21836050b6abced624f36f7acff58cef88/pkg/api/dataproxy.go#L95

@anzai
anzai commented Jul 25, 2016

@mtanda Thank you for your advice. I wonder if you mean that data proxy do NOT need to check white list with requests from sqldb frontend. Would it be right?
I noticed I have to fix this part like the case of cloudwatch. Because of the same reason as cloudwatch, "ds.Url" is always empty with sqldb datasource. So if data_source_proxy_whitelist has some values, sqldb frontend always fails to connect the backend.

490ea18

@mtanda
Contributor
mtanda commented Jul 25, 2016

@anzai Sorry for confusing by short comment.
Actually, the proxy code should move before whitelist check.
https://github.com/grafana/grafana/pull/5364/files#diff-2ce5b8fd7c5c41aeaf1e8b12a3163b80R105

And, the "sqldb" might need to check whitelist setting for "jsonData.host".
The datasource proxy whitelist is used for restricting the destination in "backend" side.
If user has "admin" priviledge for Grafana, the user can create any destination proxy setting.
It means, if Grafana doesn't restrict destination by server side, the "admin" user can access any DB server the Grafana can access (can make backdoor).

@mtanda
Contributor
mtanda commented Jul 25, 2016

For CloudWatch, we can restrict access by IAM, so Grafana don't need to check whitelist.

@anzai
anzai commented Jul 25, 2016

@mtanda Thanks. I see. I'll try to fix it.

@anzai
anzai commented Jul 25, 2016

@torkelo I think there are 3 ways to improve SQL proxy.

(1) Using xorm more generically
(2) Using ODBC
(3) Using JDBC

Most users are confused about configureing ODBC. (ex.. defining Windows DSN or Unix ODBC)
I cannot find any go-JDBC driver which is reliable enough to be included in Grafana backend.

I think (1) or (2) would be better than (3). However if someone knows a reliable JDBC driver, (3) would be the best . Which is the best do you think?

@GL133687915
GL133687915 commented Aug 11, 2016 edited

This really is a great feature !
I experienced one problem though: with the supplied test binary, it won't work with "mixed" data source.
It displays some error (Plugin Error: Failed to find exported plugin component for query-ctrl-mixed)
Is it related to the SQL feature or is it related to #4604 ?

@anzai anzai Fix to check white list when the frontend tries to connect DB
157435a
@anzai
anzai commented Aug 17, 2016

I fixed the backend not to ignore whitelist and check if sqldb's "JsonData.host" is included in the list.

@philip-wernersbach

I don't know if I should post this here or on the front end repository, so I'm posting it both places.

I've finally gotten the necessary permissions to release the InfluxDB-MySQL proxy software that I talk about in my talk here. I'm currently in the process of auditing its Git history for sensitive data, and I plan to release it later this week.

For Grafana's purposes, this pull request and the front end plugin for this makes the proxy server deprecated. But there's a few corner cases that the proxy server takes care of that I don't think this solution currently does, and both relate to differences between the way MySQL and InfluxDB handle grouping:

  1. When reporting time for groupings, MySQL does not report nice, neat times like InfluxDB does. It reports the time of one of the data points in the group.
  2. MySQL cannot perform an InfluxDB style fill for missing groupings. Instead of filling in missing time groups with NULL or zero or whatever, MySQL doesn't report them at all. Grafana currently doesn't handle this correctly.

Both of these things are really only noticeable on bar graphs. Without 1, the bar graphs won't nicely stack, and without 2, Grafana will incorrectly report bar graph times.

Anyways, both of these things are things that can be incrementally improved, I just wanted to make everyone aware of them. I'm hoping that we all can collaborate and make this pull request and the associated front end the de facto Grafana RDBMS solution.

@philip-wernersbach philip-wernersbach referenced this pull request in sraoss/grafana-sqldb-datasource Aug 18, 2016
Open

This plugin probably doesn't handle MySQL corner cases correctly #6

@anzai
anzai commented Aug 18, 2016

@GL133687915 Beta1 could have had such bug. Does that also occur with the newer binary (Beta2)?
https://github.com/sraoss/grafana-sqldb-datasource/releases/tag/V0.2_beta2

@GL133687915
GL133687915 commented Aug 18, 2016 edited

Hello Anzai,
I've tested the newer binary and I have other reactions from Grafana than before. It depends on how I use Grafana.
For the record, here's what I'm doing: I have successfully registered 2 postgresql database and I want a table comparing the results of the same query run on each DB. For this test I always do this in a 'table' panel.

  1. After creating a table panel, If I directly select "Mixed" in panel data source, I have an error message:

    Plugin Error - Failed to find exported plugin component for query-ctrl-mixed

  2. After creating a table panel, I delete the fake data source metric and select one of my DB (No mixed mode). No error.
    2.1 If I add a query on one of my DB and directly select the raw edit mode, I have an error:
    this.dataRow[0] is undefined.
    2.2 If I first play with the non-raw query editor and then switch to raw query editor, the query is perfectly executed

  3. After creating a table panel, I delete the fake data source metric and select the mixed data source. No error.
    3.1 If I add a query on one of my DB and directly select the raw edit mode, I have an error:
    this.dataRow[0] is undefined.
    3.2 If I first play with the non-raw query editor and then switch to raw query editor, I have an error:
    this.dataRow[0] is undefined.

Extra note: every time there is a this.dataRow[0] error, I have the following message in server console:

SQL request: query='[]'

Is there any other info I can provide ?

@anzai
anzai commented Aug 19, 2016 edited

@GL133687915 Thank you for your report. I fixed the frontend plugin.
sraoss/grafana-sqldb-datasource@8ccaecb

1: Plugin Error - Failed to find exported plugin component for query-ctrl-mixed

I could reproduce "Plugin Error", but I think it is by design of Table panel or a small bug.
This error occurs also with officially-relased Grafana 3.0, and with Graph panel no error occurs even if I do the same actions.

  • At the time I open the editor for new table panel, data source A is "Test metric (fake data source)".
  • When I select "--Mixed--" as "Panel data source", "Plugin Error" always occurs.
  • But if data source A isn't "Test metric" but actual data source like InfluxDB, I can choise a data source from "Add Query" combobox without any error.

2: this.dataRow[0] is undefined
3: this.dataRow[0] is undefined

I could reproduce this JS console log for 3.2. I couldn't reproduce 2.2, but they might be the same situation.
I fixed this error. I didn't consider the case of executing empty queries. in such case, an empty array was returned as the result, but the format was bad and it must have been an object.. I fixed the format.

SQL request: query='[]'

When a query haven't been generated because schema/table/time column haven't been specified yet, there is no queries to execute, and this log yields. This message was confusing, so I fixed the frontend not to try execute empty queries.

@GL133687915

@anzai
I have installed the patch on the v0.2(beta2)
The Error

this.dataRow[0] is undefined

don't show up. But in the tests I do, I always provide a valid SQL query that returns something (in raw mode).
For instance, Here are the 2 raw test queries I execute:
DB1: SELECT now() AS time, count(*) AS value, '1' AS name FROM test_table;
DB2: SELECT now() AS time, count(*) AS value, '2' AS name FROM test_table;
Each is requested to return result in table mode, not time_series.

The system only displays the result of the DB1 query. If I hide it, result of DB2 appears.

I do see each query in server console:

DBUG[08-19|10:18:11] SQL request: query='[SELECT now() AS time, count() AS value, '1' AS name FROM test_table;]'
DBUG[08-19|10:18:11] SQL request: query='[SELECT now() AS time, count(
) AS count, '2' AS name FROM test_table;]'

This problem is not happening with time-series mode: results from both DB1 & DB2 appear.

@anzai
anzai commented Aug 19, 2016

@GL133687915 Sorry, I haven't implemented the case of multiple queries in Table format and Doc format
For Table panel, the multiple result sets must be merged. But it is not necessarily that each queries have the common column to merge. So frontend should confirm if there is such common column, but I think it isn't easy.
Of course, it would be ok to merge simply by the column name. But to be more strict, even if each query have the same-named column, their data type can be different, or the column can be a subquery, etc.

@GL133687915
GL133687915 commented Aug 19, 2016 edited

@anzai
I see your point.
I have a naive question: following your idea of a merging based on column name, if the data types are different, will it mess everything up or display some error message and quietly ignore wrong results ?
In the last case, it seems to me like a normal behaviour. Or am I over simplifying the problem ?

@philip-wernersbach

Is there a database test suite for Grafana? It would be nice to adapt it for SQL and see where this stands versus InfluxDB.

@GL133687915

@anzai
I was re-thinking on what you said about multiple queries in Table format; it seems to me that such a feature would be of great interest:

  • this is the 'native format' of sql query results ;
  • we could use the 'table' panel to its full potential ;
  • plugin worldmap-panel only accept results from sqldb if they are in Table format.
    Do you think this feature can make it to your PR ?
@torkelo torkelo referenced this pull request Sep 8, 2016
Closed

Get MySQL data, to make graph ? #5988

1 of 1 task complete
@liangmingyuanneo

As a result of forking InfluxDB, whether the MySql must need a timestamp type column to support time-serise in this project?

@anzai
anzai commented Oct 13, 2016

@philip-wernersbach Do you mean if there is sample data to test the feature? If so, I haven't prepared it.
You should prepare databases and tables as you like.

@anzai
anzai commented Oct 13, 2016

@liangmingyuanneo Yes, in most case. I think most panels of Grafana need timestamp data.

@z3ugma
z3ugma commented Nov 28, 2016

@torkelo and @anzai, this is working great for me in production with the custom version of Grafana, how long until the PR is merged into the master version? What milestone should this get?

@torkelo
Member
torkelo commented Nov 28, 2016

It still needs to refactored so the backend parts are more generic and not only applicable to a single frontend plugin id. Not sure when we merge this. But hopefully in the beginning of next year.

@anzai
anzai commented Nov 29, 2016 edited

@torkelo I have no idea to refactor the backend part to meke more generic... Do you have any idea to do it?

By the way, PostgreSQL supports "FDW (foreign data wrappers)" which enables PostgreSQL servers to connect to another remote data stores (Oracle, MongoDB, Hadoop, Redis...) and gather data from them. From another point of view, using FDW feature of PostgreSQL seems to be better.

see:
https://wiki.postgresql.org/wiki/Foreign_data_wrappers

@torkelo
Member
torkelo commented Nov 29, 2016

Now only a single fronted plugin can take advantage of the backend sql proxy, that is not good

@philip-wernersbach
philip-wernersbach commented Nov 30, 2016 edited

@torkelo @anzai I've open sourced my InfluxDB-to-MySQL protocol converter and published it to https://github.com/philip-wernersbach/influx-mysql. Hopefully this can provide an interim solution for MySQL compatibility while this PR is still being refined. And the author of this PR can also check out the influx-mysql source for tips on what's necessary to do time series processing with regular SQL.

@torkelo torkelo added a commit that referenced this pull request Jan 31, 2017
@torkelo torkelo feat(): began refactoring PR #5364 01f1c1b
@alexpdp7

Shameless plug but I also wrote something similar #5364 ; it's 41 lines of code but it allows me to graph PostgreSQL tables in Grafana (you might need to create extra views to adhere to the format it requires, though).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment