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.

Sign up for GitHub

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

Jump to bottom

Add ability to set sql file as query source for postgresql_extensible input #6361

Merged
merged 4 commits into from
Sep 12, 2019
Merged

Add ability to set sql file as query source for postgresql_extensible input #6361

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
298649b
Added `script` option to be able set a sql file as a query source
Zyqsempai Sep 6, 2019
5d90d59
Readme update
Zyqsempai Sep 6, 2019
c0cbda8
Readme update + comments rework
Zyqsempai Sep 11, 2019
f744e85
added Init func
Zyqsempai Sep 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion plugins/inputs/postgresql_extensible/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ The example below has two queries are specified, with the following parameters:
# withdbname was true.
# Be careful that if the withdbname is set to false you don't have to define
# the where clause (aka with the dbname)
# Also script option can be used to speciafy the .sql file path,
glinton marked this conversation as resolved.
Show resolved Hide resolved
# in this way please make sure that sqlquery and script is not used at same time,
# coz this will cause error.
#
# the tagvalue field is used to define custom tags (separated by comas).
# the query is expected to return columns which match the names of the
Expand All @@ -61,7 +64,7 @@ The example below has two queries are specified, with the following parameters:
withdbname=false
tagvalue=""
[[inputs.postgresql_extensible.query]]
sqlquery="SELECT * FROM pg_stat_bgwriter"
script="your_sql-filepath.sql"
version=901
withdbname=false
tagvalue=""
Expand Down
27 changes: 27 additions & 0 deletions plugins/inputs/postgresql_extensible/postgresql_extensible.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package postgresql_extensible
import (
"bytes"
"fmt"
"io/ioutil"
"log"
"os"
"strings"

// register in driver.
Expand All @@ -25,6 +27,7 @@ type Postgresql struct {

type query []struct {
Sqlquery string
Script string
Version int
Withdbname bool
Tagvalue string
Expand Down Expand Up @@ -108,6 +111,20 @@ func (p *Postgresql) IgnoredColumns() map[string]bool {
return ignoredColumns
}

func (p *Postgresql) ReadQueryFromFile(filePath string) (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a non-member function.

file, err := os.Open(filePath)
if err != nil {
return "", err
}
defer file.Close()

qyery, err := ioutil.ReadAll(file)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/qyery/query/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spelling still needs fixed.

if err != nil {
return "", err
}
return string(qyery), err
}

func (p *Postgresql) Gather(acc telegraf.Accumulator) error {
var (
err error
Expand All @@ -129,8 +146,18 @@ func (p *Postgresql) Gather(acc telegraf.Accumulator) error {
// We loop in order to process each query
// Query is not run if Database version does not match the query version.
for i := range p.Query {
if p.Query[i].Sqlquery != "" && p.Query[i].Script != "" {
return fmt.Errorf("both 'sqlquery' and 'script' specified, please choose one option")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would remove this and just document that sqlquery takes preference if both are defined.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a strong opinion on returning an error vs precedence, though if I was asked to choose I think an error is slightly less prone to mistakes. However, I would like the script loading to be done only once. The easiest way to do this is to move it into a new Init() function:

func (p *Postgresql) Init() error {
    // load the file and save onto the Postgresql struct.
    // return an error if the file is unreadable or configuration is invalid
}

}
sql_query = p.Query[i].Sqlquery
if sql_query == "" {
sql_query, err = p.ReadQueryFromFile(p.Query[i].Script)
if err != nil {
return err
}
}
tag_value = p.Query[i].Tagvalue

if p.Query[i].Measurement != "" {
meas_name = p.Query[i].Measurement
} else {
Expand Down
27 changes: 26 additions & 1 deletion plugins/inputs/postgresql_extensible/postgresql_extensible_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func TestPostgresqlFieldOutput(t *testing.T) {
}

acc := queryRunner(t, query{{
Sqlquery: "select * from pg_stat_database",
Script: "test.sql",
glinton marked this conversation as resolved.
Show resolved Hide resolved
Version: 901,
Withdbname: false,
Tagvalue: "",
Expand Down Expand Up @@ -201,6 +201,31 @@ func TestPostgresqlFieldOutput(t *testing.T) {
}
}

func TestPostgresqlSqlQueryAndScriptScepifiedError(t *testing.T) {
q := query{{
Script: "test.sql",
Sqlquery: "select * from pg_stat_database",
Version: 901,
Withdbname: false,
Tagvalue: "",
}}
p := &Postgresql{
Service: postgresql.Service{
Address: fmt.Sprintf(
"host=%s user=postgres sslmode=disable",
testutil.GetLocalHost(),
),
IsPgBouncer: false,
},
Databases: []string{"postgres"},
Query: q,
}
var acc testutil.Accumulator
p.Start(&acc)

require.Error(t, acc.GatherError(p.Gather))
}

func TestPostgresqlIgnoresUnwantedColumns(t *testing.T) {
if testing.Short() {
t.Skip("Skipping integration test in short mode")
Expand Down
1 change: 1 addition & 0 deletions plugins/inputs/postgresql_extensible/test.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select * from pg_stat_database
glinton marked this conversation as resolved.
Show resolved Hide resolved