Skip to content

database_observability: don't log query text if redaction is enabled #4599

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

Merged
merged 2 commits into from
Oct 31, 2025
Merged

database_observability: don't log query text if redaction is enabled #4599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ab400d5
database_observability: don't log query text if redaction is enabled
cristiangreco Oct 10, 2025
35eb7d9
Apply suggestions from code review
cristiangreco Oct 31, 2025
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
12 changes: 6 additions & 6 deletions .../reference/components/database_observability/database_observability.postgres.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ You can use the following blocks with `database_observability.postgres`:

| Block | Description | Required |
|------------------------------------|---------------------------------------------------|----------|
| [`cloud_provider`][cloud_provider] | Provide Cloud Provider information. | no |
| `cloud_provider` > [`aws`][aws] | Provide AWS database host information. | no |
| [`cloud_provider`][cloud_provider] | Provide Cloud Provider information. | no |
| `cloud_provider` > [`aws`][aws] | Provide AWS database host information. | no |
| [`query_details`][query_details] | Configure the queries collector. | no |
| [`query_samples`][query_samples] | Configure the query samples collector. | no |
| [`schema_details`][schema_details] | Configure the schema and table details collector. | no |
Expand Down Expand Up @@ -90,10 +90,10 @@ The `aws` block supplies the [ARN](https://docs.aws.amazon.com/IAM/latest/UserGu

### `query_samples`

| Name | Type | Description | Default | Required |
|---------------------------|------------|---------------------------------------------------------|---------|----------|
| `collect_interval` | `duration` | How frequently to collect information from database. | `"15s"` | no |
| `disable_query_redaction` | `bool` | Collect unredacted SQL query text including parameters. | `false` | no |
| Name | Type | Description | Default | Required |
|---------------------------|------------|---------------------------------------------------------------|---------|----------|
| `collect_interval` | `duration` | How frequently to collect information from database. | `"15s"` | no |
| `disable_query_redaction` | `bool` | Collect unredacted SQL query text (might include parameters). | `false` | no |

### `schema_details`

Expand Down
2 changes: 1 addition & 1 deletion internal/component/database_observability/mysql/collector/query_samples.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (c *QuerySamples) Name() string {

func (c *QuerySamples) Start(ctx context.Context) error {
if c.disableQueryRedaction {
level.Warn(c.logger).Log("msg", "collector started with query redaction disabled. Query samples will include complete SQL text including query parameters.")
level.Warn(c.logger).Log("msg", "collector started with query redaction disabled. SQL text in query samples may include query parameters.")
} else {
level.Debug(c.logger).Log("msg", "collector started")
}
Expand Down
56 changes: 32 additions & 24 deletions internal/component/database_observability/postgres/collector/query_samples.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ const (
)

const (
stateActive = "active"
queryTextClause = ", s.query"
stateActive = "active"
)

const selectPgStatActivity = `
Expand All @@ -48,16 +49,16 @@ const selectPgStatActivity = `
s.wait_event,
pg_blocking_pids(s.pid) as blocked_by_pids,
s.query_start,
s.query_id,
s.query
s.query_id
%s
FROM pg_stat_activity s
JOIN pg_database d ON s.datid = d.oid AND NOT d.datistemplate AND d.datallowconn
WHERE
s.pid != pg_backend_pid() AND
s.state != 'idle' AND
(
s.backend_type != 'client backend' OR
(
(
coalesce(TRIM(s.query), '') != '' AND
s.query_id != 0
)
Expand Down Expand Up @@ -194,7 +195,11 @@ func (c *QuerySamples) Name() string {
}

func (c *QuerySamples) Start(ctx context.Context) error {
level.Debug(c.logger).Log("msg", "collector started")
if c.disableQueryRedaction {
level.Warn(c.logger).Log("msg", "collector started with query redaction disabled. SQL text in query samples may include query parameters.")
} else {
level.Debug(c.logger).Log("msg", "collector started")
}

c.running.Store(true)
ctx, cancel := context.WithCancel(ctx)
Expand Down Expand Up @@ -236,7 +241,13 @@ func (c *QuerySamples) Stop() {
}

func (c *QuerySamples) fetchQuerySample(ctx context.Context) error {
rows, err := c.dbConnection.QueryContext(ctx, selectPgStatActivity)
queryTextField := ""
if c.disableQueryRedaction {
queryTextField = queryTextClause
}

query := fmt.Sprintf(selectPgStatActivity, queryTextField)
Copy link

@semgrep-code-grafana semgrep-code-grafana bot Oct 30, 2025

Choose a reason for hiding this comment

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

Semgrep identified an issue in your code:
String-formatted SQL query detected. This could lead to SQL injection if the string is not sanitized properly. Audit this call to ensure the SQL is not manipulable by external data.

To resolve this comment:

🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by string-formatted-query.

We're currently testing semgrep's diff-aware PR comment feature on a subset of our repos-- if you run into issues or find this spammy, please reach out to @danny.cooper in slack and give feedback.

You can view more details about this finding in the Semgrep AppSec Platform.

rows, err := c.dbConnection.QueryContext(ctx, query)
if err != nil {
return fmt.Errorf("failed to query pg_stat_activity: %w", err)
}
Expand Down Expand Up @@ -279,7 +290,7 @@ func (c *QuerySamples) fetchQuerySample(ctx context.Context) error {

func (c *QuerySamples) scanRow(rows *sql.Rows) (QuerySamplesInfo, error) {
sample := QuerySamplesInfo{}
err := rows.Scan(
scanArgs := []interface{}{
&sample.Now,
&sample.DatabaseName,
&sample.PID,
Expand All @@ -300,8 +311,11 @@ func (c *QuerySamples) scanRow(rows *sql.Rows) (QuerySamplesInfo, error) {
&sample.BlockedByPIDs,
&sample.QueryStart,
&sample.QueryID,
&sample.Query,
)
}
if c.disableQueryRedaction {
scanArgs = append(scanArgs, &sample.Query)
}
err := rows.Scan(scanArgs...)
return sample, err
}

Expand All @@ -314,8 +328,10 @@ func (c *QuerySamples) processRow(sample QuerySamplesInfo) (SampleKey, error) {
}

func (c QuerySamples) validateQuerySample(sample QuerySamplesInfo) error {
if sample.Query.Valid && sample.Query.String == "<insufficient privilege>" {
return fmt.Errorf("insufficient privilege to access query. sample set: %+v", sample)
if c.disableQueryRedaction {
if sample.Query.Valid && sample.Query.String == "<insufficient privilege>" {
return fmt.Errorf("insufficient privilege to access query sample set: %+v", sample)
}
}

if !sample.DatabaseName.Valid {
Expand Down Expand Up @@ -426,13 +442,8 @@ func (c *QuerySamples) buildQuerySampleLabels(state *SampleState) string {
}
}

queryText := state.LastRow.Query.String
if !c.disableQueryRedaction {
queryText = redact(queryText)
}

labels := fmt.Sprintf(
`datname="%s" pid="%d" leader_pid="%s" user="%s" app="%s" client="%s" backend_type="%s" state="%s" xid="%d" xmin="%d" xact_time="%s" query_time="%s" queryid="%d" query="%s" engine="postgres"`,
Copy link
Contributor

@gaantunes gaantunes Oct 30, 2025

Choose a reason for hiding this comment

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

Are we removing the engine=postgres on purpose?

Copy link
Contributor Author

@cristiangreco cristiangreco Oct 31, 2025

Choose a reason for hiding this comment

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

Yes this was on purpose. Can't find the ticket anymore, but at some point we stopped using it on the frontend.

`datname="%s" pid="%d" leader_pid="%s" user="%s" app="%s" client="%s" backend_type="%s" state="%s" xid="%d" xmin="%d" xact_time="%s" query_time="%s" queryid="%d"`,
state.LastRow.DatabaseName.String,
state.LastRow.PID,
leaderPID,
Expand All @@ -446,11 +457,13 @@ func (c *QuerySamples) buildQuerySampleLabels(state *SampleState) string {
xactDuration,
queryDuration,
state.LastRow.QueryID.Int64,
queryText,
)
if state.LastCpuTime != "" {
labels = fmt.Sprintf(`%s cpu_time="%s"`, labels, state.LastCpuTime)
}
if c.disableQueryRedaction && state.LastRow.Query.Valid {
labels = fmt.Sprintf(`%s query="%s"`, labels, state.LastRow.Query.String)
}
return labels
}

Expand All @@ -460,12 +473,8 @@ func (c *QuerySamples) buildWaitEventLabels(state *SampleState, we WaitEventOccu
if state.LastRow.LeaderPID.Valid {
leaderPID = fmt.Sprintf(`%d`, state.LastRow.LeaderPID.Int64)
}
queryText := state.LastRow.Query.String
if !c.disableQueryRedaction {
queryText = redact(queryText)
}
return fmt.Sprintf(
`datname="%s" pid="%d" leader_pid="%s" user="%s" backend_type="%s" state="%s" xid="%d" xmin="%d" wait_time="%s" wait_event_type="%s" wait_event="%s" wait_event_name="%s" blocked_by_pids="%v" queryid="%d" query="%s" engine="postgres"`,
Copy link
Contributor Author

@cristiangreco cristiangreco Oct 30, 2025

Choose a reason for hiding this comment

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

Removed query text from op=WAIT_EVENT

`datname="%s" pid="%d" leader_pid="%s" user="%s" backend_type="%s" state="%s" xid="%d" xmin="%d" wait_time="%s" wait_event_type="%s" wait_event="%s" wait_event_name="%s" blocked_by_pids="%v" queryid="%d"`,
state.LastRow.DatabaseName.String,
state.LastRow.PID,
leaderPID,
Expand All @@ -480,7 +489,6 @@ func (c *QuerySamples) buildWaitEventLabels(state *SampleState, we WaitEventOccu
waitEventFullName,
we.BlockedByPIDs,
state.LastRow.QueryID.Int64,
queryText,
)
}

Expand Down
Loading
Loading