database_observability: add auto-enable setup consumers to query_sample #3922

cristiangreco · 2025-06-30T15:27:40Z

PR Description

Introduce a check in the query_sample collector to automatically enable the events_statements_cpu consumer in the performance_schema.setup_consumers table.

This is guarded by two config options:

allow_update_performance_schema_settings: to enable "updates" to performance_schema settings altogether.
auto_enable_setup_consumers: to start auto-enabling of the setting specific to the query_sample collector.

Which issue(s) this PR fixes

n.a.

Notes to the Reviewer

PR Checklist

CHANGELOG.md updated
Documentation added
Tests updated
Config converters updated

fridgepoet · 2025-07-02T02:49:32Z

internal/component/database_observability/mysql/component.go

+	DataSourceName                alloytypes.Secret   `alloy:"data_source_name,attr"`
+	CollectInterval               time.Duration       `alloy:"collect_interval,attr,optional"`
+	ForwardTo                     []loki.LogsReceiver `alloy:"forward_to,attr"`
+	EnableCollectors              []string            `alloy:"enable_collectors,attr,optional"`


I wonder, (not sure if it's possible), in the future when we have several different setup_consumers we want to configure in a fine-grained manner, will it be easier provide a list to enable/disable similar to EnableCollectors/DisableCollectors?

Might be overengineered, though, so maybe it's better to start with a simple boolean for now that is at the lowest level.

Hmm something to think about, but agree to start simple for now as we don't know how widespread this functionality will be.

fridgepoet · 2025-07-08T02:16:07Z

Should we distinguish because something that "collects" and something that will "write"?

github-actions · 2025-07-09T10:53:08Z

💻 Deploy preview deleted.

gaantunes · 2025-07-09T13:43:00Z

I wonder if we should create a separate collector that is responsible for maintaining all write operations based on enabled colletors array (taking into account defaults of course). Detaching it would allow a longer scrape period for these operations, instead of checking/writing every time we do a scrape in each collector.

cristiangreco · 2025-07-10T14:42:27Z

I wonder if we should create a separate collector that is responsible for maintaining all write operations based on enabled colletors array (taking into account defaults of course). Detaching it would allow a longer scrape period for these operations, instead of checking/writing every time we do a scrape in each collector.

Yes I agree the the two things should not be necessarily tied together. I'd prefer to postpone that for a followup PR though if that's ok (so here we first verify this is working and proves valuable, then optimise it).

cristiangreco · 2025-07-11T08:20:38Z

internal/component/database_observability/mysql/collector/query_sample.go

+	if c.autoEnableSetupConsumers {
+		if err := c.updateSetupConsumersSettings(ctx); err != nil {
+			return err
+		}
+	}
+


As commented by @gaantunes we could decouple the update from the regular collector loop. I'm keen to postpone that work for a followup PR, here I'm more interested in the correctness of the approach.

I'd be inclined to include this now, or atleast tie it to setup_consumers_collect_interval. Running this every minute seems too often

matthewnolf · 2025-07-22T09:38:38Z

internal/component/database_observability/mysql/collector/query_sample.go

+		level.Error(c.logger).Log("msg", "failed to get rows affected from performance_schema.setup_consumers", "err", err)
+		return err
+	}
+	level.Debug(c.logger).Log("msg", "updated performance_schema.setup_consumers", "rows_affected", rowsAffected)


Shouldn't we check rows affected = 1? If it's something else, the operation didn't work as expected

rowsAffected == 0 is also fine, it just means it was already enabled (that's why this is just a debug log)

matthewnolf · 2025-07-22T09:39:06Z

internal/component/database_observability/mysql/collector/query_sample.go

+
+	rowsAffected, err := rs.RowsAffected()
+	if err != nil {
+		level.Error(c.logger).Log("msg", "failed to get rows affected from performance_schema.setup_consumers", "err", err)


Are we going to end up double logging this error? Here and in the ticker loop

Good point, I'll change these to be fmt.Errorf rather than to log here.

matthewnolf · 2025-07-22T09:40:35Z

internal/component/database_observability/mysql/collector/query_sample.go

+	if c.autoEnableSetupConsumers {
+		if err := c.updateSetupConsumersSettings(ctx); err != nil {
+			return err
+		}
+	}
+


I'd be inclined to include this now, or atleast tie it to setup_consumers_collect_interval. Running this every minute seems too often

clayton-cornell

Approving for docs

Introduce a check in the `query_sample` collector to automatically enable the `events_statements_cpu` consumer in the `performance_schema.setup_consumers` table. This is guarded by two config options: - `allow_update_performance_schema_settings`: to enable "updates" to performance_schema settings altogether. - `auto_enable_setup_consumers`: to start auto-enabling of the setting specific to the `query_sample` collector.

cristiangreco force-pushed the cristian/dbo11y-query-sample-autoenable-cpu-consumer branch from 941c4f7 to 7219121 Compare June 30, 2025 16:01

fridgepoet reviewed Jul 2, 2025

View reviewed changes

fridgepoet approved these changes Jul 8, 2025

View reviewed changes

cristiangreco force-pushed the cristian/dbo11y-query-sample-autoenable-cpu-consumer branch 3 times, most recently from d8e5886 to d5a6d9a Compare July 9, 2025 10:51

cristiangreco force-pushed the cristian/dbo11y-query-sample-autoenable-cpu-consumer branch from d5a6d9a to eec1c14 Compare July 9, 2025 10:54

cristiangreco force-pushed the cristian/dbo11y-query-sample-autoenable-cpu-consumer branch 3 times, most recently from 221ac06 to 623ef02 Compare July 10, 2025 14:37

cristiangreco commented Jul 11, 2025

View reviewed changes

cristiangreco marked this pull request as ready for review July 11, 2025 08:20

cristiangreco requested review from matthewnolf, clayton-cornell and a team as code owners July 11, 2025 08:20

matthewnolf reviewed Jul 22, 2025

View reviewed changes

clayton-cornell approved these changes Jul 22, 2025

View reviewed changes

cristiangreco added 2 commits July 29, 2025 15:45

remove log

146397e

cristiangreco force-pushed the cristian/dbo11y-query-sample-autoenable-cpu-consumer branch from 623ef02 to 146397e Compare July 29, 2025 13:56

run check async

deb7ad5

gaantunes approved these changes Jul 29, 2025

View reviewed changes

matthewnolf approved these changes Jul 30, 2025

View reviewed changes

cristiangreco merged commit 64165ff into main Jul 30, 2025
41 checks passed

cristiangreco deleted the cristian/dbo11y-query-sample-autoenable-cpu-consumer branch July 30, 2025 11:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

database_observability: add auto-enable setup consumers to query_sample #3922

database_observability: add auto-enable setup consumers to query_sample #3922

Uh oh!

cristiangreco commented Jun 30, 2025

Uh oh!

fridgepoet Jul 2, 2025

Uh oh!

cristiangreco Jul 9, 2025

Uh oh!

fridgepoet commented Jul 8, 2025

Uh oh!

github-actions bot commented Jul 9, 2025 •

edited

Loading

Uh oh!

gaantunes commented Jul 9, 2025

Uh oh!

cristiangreco commented Jul 10, 2025

Uh oh!

cristiangreco Jul 11, 2025

Uh oh!

matthewnolf Jul 22, 2025

Uh oh!

matthewnolf Jul 22, 2025

Uh oh!

cristiangreco Jul 29, 2025

Uh oh!

matthewnolf Jul 22, 2025

Uh oh!

cristiangreco Jul 29, 2025

Uh oh!

matthewnolf Jul 22, 2025

Uh oh!

clayton-cornell left a comment

Uh oh!

Uh oh!

Uh oh!

database_observability: add auto-enable setup consumers to query_sample #3922

database_observability: add auto-enable setup consumers to query_sample #3922

Uh oh!

Conversation

cristiangreco commented Jun 30, 2025

PR Description

Which issue(s) this PR fixes

Notes to the Reviewer

PR Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fridgepoet commented Jul 8, 2025

Uh oh!

github-actions bot commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaantunes commented Jul 9, 2025

Uh oh!

cristiangreco commented Jul 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clayton-cornell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 9, 2025 •

edited

Loading