Skip to content
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

Genericsqlplugin #2785

Closed
wants to merge 25 commits into from
Closed

Conversation

lucadistefano
Copy link

The plugin executes simple queries or query scripts on multiple servers.
It permits to select the tags and the fields to export, if is needed fields can be forced to a choosen datatype.
Supported drivers are go-mssqldb (sqlserver) , oci8 ora.v4 (Oracle), mysql (MySQL), pq (Postgres)

it is highly flexible and can do both type of counters gathering: horizontal and vertical .
horizontal: where each counter is on different columns of a row and the counter name is the name of the column
vertical : where each counter is on different row and the counter name is value of a specified column

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

fixed initialization
conversion to string of tag values
tested oracle,sqlserver,mysql
conversion to string of tag values
tested oracle,sqlserver,mysql
@anilkulkarni87
Copy link

Have you tried with a postgresql DB? Is this plugin that something i can try to use with Redshift?

@danielnelson
Copy link
Contributor

@lucadistefano Can you review #2286

@lucadistefano
Copy link
Author

the plugin potentially can work with all drivers that implements the database/sql api
https://github.com/golang/go/wiki/SQLDrivers

for postgres I can try in the next days, it should be sufficient set driver="pq"
for redshift I cannot test it

@anilkulkarni87
Copy link

@lucadistefano I would like to test it for Redshift. Is this going to be merged with master?

@danielnelson
Copy link
Contributor

This has a lot of overlap with #2286. We need to converge on a single plugin before we can consider it for inclusion.

@lucadistefano
Copy link
Author

yes I agree with you there is the need to converge in a single plugin.
In #2286 I found some limitations:

  1. some go sql driver implementations not converts sql datatypes into go datatypes as expected (see oci8) and sometime you have to force datatype of fields. Without this is difficult to support multiple databases in a flexible way
  2. queries for gather database performance data can be horribly long and complex. you cannot write it in the tool, you need to define it in an external sql script
  3. for performance optimisation you need to offer to keep open the database connection and reuse prepared statements instead of to redo the parse work at each poll
  4. if you need to gather performance counters of a database often you have the data in a 'vertical' structure where the name of counter is specified in a cell of the row. you can write an sql script that 'rotate' the structure or let the plugin support natively this
  5. sometimes the timestamp of the real (inside the db) poll of performance counters is stored in the performance tables and could be not correct to use the timestamp of the plugin poll
  6. if you have a cluster of databases you need to perform the same queries (can be many and many..) for the whole cluster, so the plugin should handle the poll with same queries on different databases server

I tried to solve all those limitations.
In #2286 there is a check for database version that I have not implemented here, If needed can be merged.

any feedback is wellcome!

@lucadistefano
Copy link
Author

@danielnelson
I run a simple test using the following config:

[[inputs.sql]]
	#debug=true
	interval = "20s"
	driver = "postgres"
	keep_connection=true
	servers  = ["postgres://user:password@pghost/dbname?sslmode=disable"]
	hosts=["pghost.my.lan"]
	## Queries to perform
	[[inputs.sql.query]]
		query="SELECT * FROM pg_stat_database"
		measurement="pg_stat_database"
		tag_cols=["datname", "datid"]

the result is:

> pg_stat_database,host=pghost.my.lan,datid=10793,datname=postgres numbackends=0i,xact_commit=0i,xact_rollback=0i,blks_read=0i,blks_hit=0i 1494575775000000000
> pg_stat_database,host=pghost.my.lan,datid=16384,datname=rue numbackends=0i,xact_commit=0i,xact_rollback=0i,blks_read=0i,blks_hit=0i 1494575775000000000
> pg_stat_database,host=pghost.my.lan,datid=1,datname=template1 numbackends=0i,xact_commit=0i,xact_rollback=0i,blks_read=0i,blks_hit=0i 1494575775000000000
> pg_stat_database,host=pghost.my.lan,datid=10792,datname=template0 numbackends=0i,xact_commit=0i,xact_rollback=0i,blks_read=0i,blks_hit=0i 1494575775000000000

The merge is under discussion.

implemented null_as_zero for all datatypes
removed duplicated log for error in conversion
now the field_value column can be forced to a choosen datatype
beautified log
@lucadistefano
Copy link
Author

I found a way for to use drivers with external shared lib deps without to link it in telegraf:

Oracle driver with golang 1.8 plugins feature

Follow the docu in https://github.com/mattn/go-oci8 for build the oci8 driver.
If all is going well now golang oci8 driver is compiled and linked against oracle shared libs. But not linked in telegraf.

For let i use in telegraf, do the following:
create a file plugin.go with this content:

package main

import "C"

import (
	"log"
	"fmt"
	"github.com/mattn/go-oci8"
	// .. here you can agg other proprietary driver
)

func main() {
	o := oci8.OCI8Driver{}
	log.Printf("I! Loaded shared lib '%s'", fmt.Sprintf("%v", o))
}

build it with

mkdir $GOPATH/lib
go build -buildmode=plugin -o $GOPATH/lib/oci8_go.so plugin.go

in the input plugin configuration specigy the path of the created shared lib

[[inputs.sql]]
	...
	driver = "oci8"
	shared_lib = "/home/luca/.gocode/lib/oci8_go.so"
	...

The steps of above can be reused in a similar way for other proprietary and non proprietary drivers

better error handling and logging
added option for get measurement name from row
fixed bug in int64 to str
added option for ignore errors on row parsing
@lucadistefano
Copy link
Author

@danielnelson can I do something for help go further with the PR?
thanks!

@jgeth
Copy link

jgeth commented May 26, 2020

only a few minor changes were required in the original code.

@baburajvelayudhan Do you have a public fork you can share those changes? Would be happy to review or contribute to get this moving forward. Personally, I would prefer a minimally-viable solution with simple table queries for now.

@danielnelson danielnelson self-assigned this Jun 8, 2020
@danielnelson danielnelson modified the milestones: 1.15.0, 1.16.0 Jun 26, 2020
@ddemin
Copy link

ddemin commented Aug 12, 2020

Really need this!

@sjwang90
Copy link
Contributor

sjwang90 commented Sep 9, 2020

@baburajvelayudhan do you mind sharing any public fork you may have for others to review and contribute? Would love to see how close we are to possibly getting this plugin in.

@sjwang90 sjwang90 modified the milestones: 1.16.0, Planned Sep 14, 2020
@MartynKeigher
Copy link

@baburajvelayudhan - Did you get any further with your request!? Would like to try it out also!
Thanks.
://mk

@danielnelson danielnelson removed their assignment Oct 21, 2020
@sjwang90 sjwang90 added wip and removed feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin labels Nov 11, 2020
@sjwang90 sjwang90 mentioned this pull request Nov 23, 2020
3 tasks
@sjwang90 sjwang90 mentioned this pull request Dec 15, 2020
3 tasks
@jinnatar
Copy link

Any updates on this one? All other FRs are being closed in favor of it so would be nice to know where we stand.

@baburajvelayudhan
Copy link

I don't work anymore with telegraf and unfortunately i am not in a position to continue work on this due to my current job committments.

@jinnatar
Copy link

@sjwang90 Are you working on this one since you added the WIP label? Just want the situation transparent since no one is assigned right now.

@sjwang90
Copy link
Contributor

sjwang90 commented Jan 4, 2021

@Artanicus Nope I'm not working on it. There were previously others involved. If you're interested in taking this on please go ahead.

@sjwang90 sjwang90 removed the wip label Jan 4, 2021
@Hipska Hipska added the plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins label Jan 21, 2021
@sjwang90 sjwang90 removed this from the Planned milestone Jan 29, 2021
@sjwang90 sjwang90 linked an issue Jan 29, 2021 that may be closed by this pull request
@sjwang90 sjwang90 added help wanted Request for community participation, code, contribution and removed help wanted Request for community participation, code, contribution labels Mar 8, 2021
@sjwang90 sjwang90 mentioned this pull request Mar 10, 2021
2 tasks
@srebhan
Copy link
Member

srebhan commented Jun 16, 2021

Hey guys, with merging #8735 we are now having a generic SQL input plugin in master and, if everything goes well, in 1.19. Please, if you can spend some time, test the current master! If you find problems or miss some features, please create an issue or cook-up a PR.
Let's close this PR and work on the in-tree plugin.

@srebhan srebhan closed this Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mysql area/postgresql area/sqlserver new plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generic DB Query Plugin (DBI plugin)