Skip to content

[pg_stat_statements] Strip special characters from query - #1166

Open
ussrlongbow wants to merge 1 commit into
prometheus-community:masterfrom
almaops:feature/pg_stat_statements_strip_special
Open

[pg_stat_statements] Strip special characters from query#1166
ussrlongbow wants to merge 1 commit into
prometheus-community:masterfrom
almaops:feature/pg_stat_statements_strip_special

Conversation

@ussrlongbow

Copy link
Copy Markdown

Same queries from Windows and Linux clients to PostgreSQL may appear as different texts in pg_stat_statements.query having same query ID, because of different line breaks, which blocks proper matching query text to metrics with queryid label in Prometheus.
Example:

# Series from two replicas, one used by Windows client, another by Linux
$ curl -s 'http://prometheus.local:9090/api/v1/series?match[]=pg_stat_statements_query_id%7Bqueryid=%220000000000%22%7D' | jq .data.[].query 
"SELECT\r\n    "Username"\r\nFROM\r\n    \"Users\"\r\nWHERE\r\n        \"UserID\" = $1"
"SELECT\n    "Username"\nFROM\n    \"Users\"\nWHERE\n        \"UserID\" = $1"

This pull request adds option --[no-]collector.stat_statements.strip_special to remove such symbols from query output.

Authors:
@ussrlongbow
@freehck

@ArthurSens ArthurSens left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey, sorry for the very delayed review here. We'll need to rebase this PR to continue 😬

Default(fmt.Sprintf("%v", defaultDisabled)).
Bool()
statementStripSpecialFlag = kingpin.Flag(
fmt.Sprint(collectorFlagPrefix, statStatementsSubsystem, ".strip_special"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

strip_special is quite generic when all we do here is strip white spaces.

I'm also wondering if this should be configurable at all; maybe the fix should be applied automatically based on GOOS?


const (
pgStatStatementQuerySelect = `LEFT(pg_stat_statements.query, %d) as query,`
pgStatStatementQuerySelectStrip = `LEFT(regexp_replace(pg_stat_statements.query, '\s+', ' ', 'g'), %d) as query,`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

\s+ strips a lot more than just CRLF; it strips multiple spaces, tabs, and new lines. Should we use a more restrictive regex?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants