Fix MySQL statement metrics crash on embedded null character - #25281
Draft
joeykelroy wants to merge 2 commits into
Draft
joeykelroy wants to merge 2 commits into
joeykelroy wants to merge 2 commits into
Conversation
Rows unioned in from performance_schema.prepared_statements_instances carry raw sql_text, which may contain an embedded null character. Obfuscation raised ValueError on it, and the except handler then logged the same raw text, which raised again inside the log handler and crashed the statement-metrics job loop. The job restarted every check run and crashed again, so no query metrics were collected at all. Pass replace_null_character=True so the row is collected instead of dropped, and use repr() when logging the offending text so the error handler cannot raise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
❌ Dispatcher tests · failed
Caution Dispatcher tests failed. See the failures below.
Batches
❌ Failures
|
evalya-impact-summaryevalya impact analysis |
Contributor
Validation ReportAll 21 validations passed. Show details
|
Contributor
|
✅ All CI checks and tests passed. Datadog automation helped this PR pass. 🎉 All green!🧪 All tests passed 🔄 Datadog retried 1 test - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: 755d454 | Docs | View more details | Give us feedback! |
Contributor
Disk usage changeCommit Uncompressed
Details
Compressed
Details
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes a crash loop in the MySQL statement-metrics job when a query text contains an embedded null character (
\x00)._normalize_queriesfailed in two stages:obfuscate_sql_with_metadataraisedValueError: embedded null characteron the row.excepthandler logged the same raw text, and becauseAgentLogHandler.emitpasses the formatted message into the Go binding, that log call raised the identicalValueErroragain.The second exception escaped the handler and crashed the whole job loop, so the job restarted on every check run and crashed again rather than skipping the one bad row. No query metrics were collected at all.
This PR:
replace_null_character=True, so the row is collected with the null character removed instead of being dropped. This reuses the existing base-check option added for SQL Server in [DBMON-3495] Replace embedded null characters in query text #16742.repr()for the offending text in the warning, so the error handler cannot itself raise.obfuscate_sql_with_metadatadocstring, which described embedded null characters as a SQL Server trait.Motivation
Reported in SDBM-2979: a customer's statement-metrics job crash-looped every 15s (344 crashes in the flare window) on MySQL 8.0.30, with complete loss of query metrics. Activity samples, query samples and metadata collection were all unaffected.
The null character does not come from
events_statements_summary_by_digest. It cannot: literals are normalized to?, comments are stripped, MySQL forbidsU+0000in identifiers, and unparseable statements get no digest row. I verified each of those.It comes from
performance_schema.prepared_statements_instances.sql_text, which #21425 unioned into the same query asAS digest_text. That column is raw, un-normalized statement text, so a null character in a literal survives. The alias meant unsafe data began flowing through a variable whose name implied the database had already normalized it, and_normalize_querieswas not revisited.This is also why only statement-metrics crashed:
statement_samples.pyandactivity.pygate their raw-text log behindlog_unobfuscated_queries(default off), so they take the safedebugbranch.statements.pyhas no such gate.Verified against a local reproduction (MySQL 8.0.30 + Agent 7.83.1, a null-bearing prepared statement created via
COM_STMT_PREPAREand held open):ddev env start --dev: check[OK], statement-metrics job started once and never crashed, 378 query-metric submissions and climbing, with the null-bearing statement live and executing throughout.The regression test fails without the fix (
'abc\x00def'vs'abcdef') and passes with it. Postgres is not affected: its wire protocol null-terminates strings, so a null character truncates the query into a syntax error andchr(0)is rejected outright, meaning it can never reachpg_stat_statements.query.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged