Skip to content

database_observability: use tokenizer to extract table names as fallback #3968

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cristiangreco
Copy link
Contributor

PR Description

Improve parsing of table names from sql statements by using go-sqllexer as a fallback when the primary parser fails.

This means that there are chances we log a partial list of table names when the sql statement is truncated, but we're heading towards making alloy more resilient and mark statements as deemed unparseable.

Which issue(s) this PR fixes

n.a.

Notes to the Reviewer

PR Checklist

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

Improve parsing of table names from sql statements by using go-sqllexer
as a fallback when the primary parser fails.

This means that there are chances we log a partial list of table names
when the sql statement is truncated, but we're heading towards making
alloy more resilient and mark statements as deemed unparseable.
@cristiangreco cristiangreco force-pushed the cristian/dbo11y-sql-tokenizer branch from bd1d0c4 to 6704ba0 Compare July 9, 2025 09:25
@cristiangreco cristiangreco marked this pull request as ready for review July 9, 2025 09:36
@cristiangreco cristiangreco requested review from matthewnolf, fridgepoet and a team as code owners July 9, 2025 09:36
@gaantunes
Copy link
Contributor

I wonder if this new parser has the same issue that lists CTE tables as actual tables?

@cristiangreco
Copy link
Contributor Author

I wonder if this new parser has the same issue that lists CTE tables as actual tables?

As chatted, CTE tables appear only in the list of tables touched by a particular query, not in the global list of tables.

But for a more direct answer, yes CTE tables should still be detected by the lexer.

@gaantunes
Copy link
Contributor

I wonder if this new parser has the same issue that lists CTE tables as actual tables?

As chatted, CTE tables appear only in the list of tables touched by a particular query, not in the global list of tables.

But for a more direct answer, yes CTE tables should still be detected by the lexer.

We can seek classifying these as temp tables in a future iteration.

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.

4 participants