Skip to content

dbo11y: stop tracking alloy's own queries in mysql#4978

Merged
cristiangreco merged 7 commits intomainfrom
cristian/dbo11y-disable-own-queries-tracking
Dec 4, 2025
Merged

dbo11y: stop tracking alloy's own queries in mysql#4978
cristiangreco merged 7 commits intomainfrom
cristian/dbo11y-disable-own-queries-tracking

Conversation

@cristiangreco
Copy link
Contributor

@cristiangreco cristiangreco commented Dec 2, 2025

PR Description

This PR introduces a change for excluding "own queries" in mysql: we add a new setup_actors collector that checks (and optionally updates) settings for the currently connected user. By default it just checks that enabled=NO and logs out if that's not the case. Optionally, the component can auto-update this setting itself. This behaviour is disabled by default, though: it follows the same pattern of setup_consumers where two boolean settings are required to enable the auto-update. Additionally, auto-update requires INSERT and UPDATE permissions (docs on the website).

Which issue(s) this PR fixes

Relates to https://github.com/grafana/grafana-dbo11y-app/issues/1307

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@github-actions
Copy link
Contributor

github-actions bot commented Dec 2, 2025

💻 Deploy preview available (dbo11y: stop tracking alloy's own queries in mysql and postgres):

@cristiangreco cristiangreco force-pushed the cristian/dbo11y-disable-own-queries-tracking branch 2 times, most recently from cbaeaeb to 0221c8d Compare December 2, 2025 19:07
@cristiangreco cristiangreco marked this pull request as ready for review December 2, 2025 19:16
@cristiangreco cristiangreco requested review from a team and clayton-cornell as code owners December 2, 2025 19:16
@cristiangreco cristiangreco changed the title dbo11y: stop tracking alloy's own queries in mysql and postgres dbo11y: stop tracking alloy's own queries in mysql Dec 3, 2025
@fridgepoet
Copy link
Contributor

Neat!
This other PR #4991 is similar, but in Postgres, right? I am wondering if the collector names should reflect the outcome rather than the underlying settings names.
If in both cases we're not tracking alloy's queries, should the collectors be named after their purpose?

@cristiangreco
Copy link
Contributor Author

Neat! This other PR #4991 is similar, but in Postgres, right?

Yes, more details in the ticket

I am wondering if the collector names should reflect the outcome rather than the underlying settings names. If in both cases we're not tracking alloy's queries, should the collectors be named after their purpose?

Yeah I see the issue, I'm not sure what a good name could be though. I guess we might have more of these kind of "check-adjust-report" functionality in the future, and maybe they shouldn't be collectors (or maybe it's ok, if they also report data?).

@fridgepoet
Copy link
Contributor

fridgepoet commented Dec 3, 2025

Agreed, calling this a "collector" is also a little disorienting.
At least for Postgres #4991 exclude_current_user is somewhat self-explanatory.
Mostly raised my comment just to highlight how "user-friendly" we are being for this. I don't know if users will know of the top of their head what "setup_actors" is and even I will likely not remember forever.

Also I linked #4991 to question if the two collectors should have the same name if they share the same end goal?

(Maybe something like exclude_own_queries or exclude_self_queries?)

@cristiangreco
Copy link
Contributor Author

I don't know if users will know of the top of their head what "setup_actors" is and even I will likely not remember forever.
Also I linked #4991 to question if the two collectors should have the same name if they share the same end goal?

Don't think should have the same name, they do different things, and trying to make things too homogeneous across different engines is counter-productive imo. TBH there's a risk we might be overthinking this: as long as our docs explain what the collector is used for, it's not that much important trying to find an alternative to the name "collector" (at this stage, at least).


var enabled, history string
err = c.dbConnection.QueryRowContext(ctx, selectQuery, user).Scan(&enabled, &history)
if err == sql.ErrNoRows {
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be an errors.Is()

| `schema_details` | Collect schemas and tables from `information_schema`. | yes |
| `query_samples` | Collect query samples. | yes |
| `setup_consumers` | Collect enabled `performance_schema.setup_consumers`. | yes |
| `setup_actors` | Check and update `performance_schema.setup_actors` settings. | yes |
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like it is enabled by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's in component.go:

collector.SetupActorsCollector: true


func (c *SetupActors) checkSetupActors(ctx context.Context) error {
var user string
err := c.dbConnection.QueryRowContext(ctx, selectUserQuery).Scan(&user)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be done on startup? Is there a need to get the current user every run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I can move this operation to before entering the loop.

As an edge case, you could change the user in the DSN string and reload the Alloy config - but that should restart the component and all collectors, so should be ok.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

💻 Deploy preview deleted (dbo11y: stop tracking alloy's own queries in mysql).

This PR introduces strategies for excluding "own queries" in mysql and
postgres:
- postgres: here we just need to drop samples (and wait events) for the
currently connected user when scraping `pg_stat_activity`. This new
behaviour is enabled by default, as it's deemed safe/recommended.
No changes to be done on `pg_stat_statements` as that is achieved
through user permissions update (docs on the website).
- mysql: here we add a new `setup_actors` collector that checks
(and optionally updates) settings for the currently connected user.
By default it just checks that `enabled=NO` and logs out if that's not
the case. Optionally, the component can auto-update this setting itself.
This behaviour is disabled by default, though: it follows the same
pattern of `setup_consumers` where two boolean settings are required to
enable the auto-update. Additionally, auto-update requires `INSERT` and
`UPDATE` permissions (docs on the website).
@cristiangreco cristiangreco force-pushed the cristian/dbo11y-disable-own-queries-tracking branch from f18eac9 to b6c0cc1 Compare December 4, 2025 07:33
…ase_observability.mysql.md

Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com>
@cristiangreco cristiangreco merged commit 1d34414 into main Dec 4, 2025
44 checks passed
@cristiangreco cristiangreco deleted the cristian/dbo11y-disable-own-queries-tracking branch December 4, 2025 09:37
dehaansa pushed a commit to madhub/alloy that referenced this pull request Dec 10, 2025
dbo11y: stop tracking alloy's own queries in mysql and postgres

This PR introduces a change for excluding "own queries" in mysql: we add a new `setup_actors` collector that checks (and optionally updates) settings for the currently connected user. By default it just checks that `enabled=NO` and logs out if that's not the case. Optionally, the component can auto-update this setting itself. This behaviour is disabled by default, though: it follows the same pattern of `setup_consumers` where two boolean settings are required to enable the auto-update. Additionally, auto-update requires `INSERT` and `UPDATE` permissions (docs on the website).
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants