*: support logging session connect attrs to slow query log#66617
*: support logging session connect attrs to slow query log#66617jiong-nba wants to merge 7 commits intopingcap:masterfrom
Conversation
ti-chi-bot
bot
added
do-not-merge/needs-tests-checked
release-note-none
|
Review Complete Findings: 2 issues |
ti-chi-bot
bot
added
the
size/XL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
|
|
Hi @jiong-nba. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCapture, parse, store, and expose client Session_connect_attrs: handshake decoding with limits/truncation/metrics, new sysvar and status entries, slow-query emission and parsing of Session_connect_attrs, and tests across server, executor, and session packages. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant HandshakeParser as Handshake Parser
participant Validator as Validation & Truncation
participant SessionCtx as Session Context
participant SlowLog as Slow Log Emitter
participant InfoSchema as Slow Query Store
Client->>HandshakeParser: send handshake with connect attrs
HandshakeParser->>Validator: pass raw attrs bytes
rect rgba(220, 180, 120, 0.5)
alt total size > 1 MiB
Validator-->>HandshakeParser: return error (hard limit)
HandshakeParser-->>Client: reject connection
else within hard limit
alt exceeds configured ConnectAttrsSize
Validator->>Validator: truncate accepted attrs, set "_truncated"
Validator->>SessionCtx: record accepted attrs
Validator->>SessionCtx: ConnectAttrsLost += truncated_bytes
else accept all attrs
Validator->>SessionCtx: store attrs
end
alt totalSize < 64 KiB
Validator->>Validator: update ConnectAttrsLongestSeen
end
HandshakeParser-->>Client: handshake OK
end
end
Client->>SlowLog: execute query
SlowLog->>SessionCtx: read SessionConnectAttrs
SlowLog->>SlowLog: encode attrs as JSON
SlowLog->>InfoSchema: write slow log row with Session_connect_attrs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR enhances TiDB’s slow query logging and slow-log system tables by capturing and exposing MySQL client SESSION_CONNECT_ATTRS (connection attributes) to better identify client/application sources in dynamic environments (e.g., Kubernetes).
Changes:
- Add
Session_connect_attrsto the slow log output (JSON) and expose it viaSLOW_QUERY/CLUSTER_SLOW_QUERYas a JSON column. - Implement a functional global system variable
performance_schema_session_connect_attrs_sizeplus global status counters to track truncation and largest seen payload. - Enforce handshake parsing limits (1 MiB hard cap + configurable soft cap) and add tests for truncation/rejection and slow-log parsing/formatting.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/sessionctx/variable/tests/session_test.go | Adds test assertions for slow log serialization and placement of Session_connect_attrs. |
| pkg/sessionctx/variable/sysvar.go | Implements performance_schema_session_connect_attrs_size as a real GLOBAL sysvar backed by an atomic. |
| pkg/sessionctx/variable/statusvar.go | Adds GLOBAL status variables and wires them to atomic counters. |
| pkg/sessionctx/variable/slow_log.go | Adds Session_connect_attrs slow log field and JSON serialization in slow log formatter. |
| pkg/sessionctx/variable/noop.go | Removes the previous noop definition of performance_schema_session_connect_attrs_size. |
| pkg/sessionctx/vardef/tidb_vars.go | Adds defaults and atomic counters for connect-attrs size/metrics. |
| pkg/sessionctx/vardef/sysvar.go | Adds vardef constant for performance_schema_session_connect_attrs_size. |
| pkg/server/internal/parse/parse.go | Adds hard/soft limits, truncation accounting, and warning emission during handshake attrs parsing. |
| pkg/server/conn_test.go | Adds handshake parsing tests for truncation, hard-limit rejection, and longest-seen behavior. |
| pkg/infoschema/tables.go | Adds JSON column for Session_connect_attrs to slow query system tables. |
| pkg/executor/slow_query_test.go | Updates expected record strings and adds a parser test for Session_connect_attrs JSON. |
| pkg/executor/slow_query.go | Parses Session_connect_attrs field and maps it to a JSON datum for system table output. |
| pkg/executor/adapter_slow_log.go | Plumbs SessionConnectAttrs from SessionVars.ConnectionInfo.Attributes into slow log items. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
pkg/server/internal/parse/parse.go (1)
136-141: Add an explicit bounds check before slicing the attrs payload.Line 140 can still hit panic/recover for malformed packets where declared
numis within 1 MiB but exceeds remaining bytes. Prefer returningmysql.ErrMalformPacketdirectly.♻️ Proposed hardening
if num, null, intOff := util2.ParseLengthEncodedInt(data[offset:]); !null { if num > 1<<20 { // 1 MiB hard limit return errors.New("connection refused: session connection attributes exceed the 1 MiB hard limit") } offset += intOff // Length of variable length encoded integer itself in bytes + if int(num) > len(data)-offset { + return mysql.ErrMalformPacket + } row := data[offset : offset+int(num)] attrs, warning, err := parseAttrs(row)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/server/internal/parse/parse.go` around lines 136 - 141, The slice operation at row := data[offset : offset+int(num)] can panic if num exceeds remaining bytes even when under the 1 MiB limit; before slicing, check that offset+int(num) <= len(data) and if not return mysql.ErrMalformPacket (or the package's equivalent) instead of letting a panic occur, preserving existing behavior for parseAttrs and using the same local symbols (num, offset, intOff, parseAttrs) to locate and harden the bounds check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/executor/slow_query_test.go`:
- Around line 287-341: Add an integration test in slow_query_sql_test.go (e.g.
TestSlowQuerySessionConnectAttrs) that exercises SESSION_CONNECT_ATTRS via SQL:
create a temp slow log file containing the same slowLogStr used in
TestParseSlowLogSessionConnectAttrs, initialize/reload the slow-query retriever
(reuse newSlowQueryRetriever/parseLog or the service initialization path used by
other slow_query_sql tests) so the slow log row is visible to
INFORMATION_SCHEMA.SLOW_QUERY, then run a TestKit query like SELECT
SESSION_CONNECT_ATTRS FROM INFORMATION_SCHEMA.SLOW_QUERY WHERE Digest=... (or
other identifying filter) and assert the returned JSON contains
"_client_name","Go-MySQL-Driver","_os","linux","app_name","test_app"; ensure
cleanup of the temp file and any retriever state.
In `@pkg/server/internal/parse/parse.go`:
- Around line 213-221: Reject or strip any client-supplied "_truncated" connect
attribute when building attrs and only set attrs["_truncated"] internally when
truncated == true; specifically, during attribute parsing/ingestion ensure you
ignore keys equal to "_truncated" (or rename the internal marker to a clearly
reserved constant and remove client-provided values) so that the only time
attrs["_truncated"] appears is when you assign truncatedBytes = totalSize -
acceptedSize and set attrs["_truncated"] = strconv.FormatInt(truncatedBytes, 10)
from the truncation branch (the symbols to locate are attrs, "_truncated",
truncated, truncatedBytes, totalSize, acceptedSize).
In `@pkg/sessionctx/variable/tests/session_test.go`:
- Around line 359-363: The test currently checks that "Session_connect_attrs"
(attrsIdx) appears before the SQL but does not ensure it appears after
"Storage_from_mpp"; update the assertions to locate storageIdx :=
strings.Index(logString, "Storage_from_mpp") and then require that attrsIdx >
storageIdx and attrsIdx < sqlIdx (using attrsIdx, storageIdx, sqlIdx and the
existing logString/sql variables) so the ordering "Storage_from_mpp" ->
"Session_connect_attrs" -> SQL is enforced.
---
Nitpick comments:
In `@pkg/server/internal/parse/parse.go`:
- Around line 136-141: The slice operation at row := data[offset :
offset+int(num)] can panic if num exceeds remaining bytes even when under the 1
MiB limit; before slicing, check that offset+int(num) <= len(data) and if not
return mysql.ErrMalformPacket (or the package's equivalent) instead of letting a
panic occur, preserving existing behavior for parseAttrs and using the same
local symbols (num, offset, intOff, parseAttrs) to locate and harden the bounds
check.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
pkg/executor/adapter_slow_log.gopkg/executor/slow_query.gopkg/executor/slow_query_test.gopkg/infoschema/tables.gopkg/server/conn_test.gopkg/server/internal/parse/parse.gopkg/sessionctx/vardef/sysvar.gopkg/sessionctx/vardef/tidb_vars.gopkg/sessionctx/variable/noop.gopkg/sessionctx/variable/slow_log.gopkg/sessionctx/variable/statusvar.gopkg/sessionctx/variable/sysvar.gopkg/sessionctx/variable/tests/session_test.go
💤 Files with no reviewable changes (1)
- pkg/sessionctx/variable/noop.go
![pantheon-ai[bot]](https://avatars.githubusercontent.com/in/2265341?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #66617 +/- ##
================================================
- Coverage 77.6714% 77.1013% -0.5702%
================================================
Files 2008 1929 -79
Lines 549230 543840 -5390
================================================
- Hits 426595 419308 -7287
- Misses 120964 123964 +3000
+ Partials 1671 568 -1103
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
/retest |
|
@jiong-nba: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
jiong-nba
force-pushed
the
feature/slow-log-session-attrs
branch
from
1217fe1 to
67a951b
Compare
ti-chi-bot
bot
added
size/XXL
/retest |
|
@jiong-nba: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test all |
|
@jiong-nba: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
jiong-nba
force-pushed
the
feature/slow-log-session-attrs
branch
from
67a951b to
4e68a10
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
pkg/executor/slow_query_sql_test.go (1)
582-588: Avoid direct type assertion for JSON cell extraction in test assertions.
rows[0][0].(string)can panic if result encoding changes. A safer conversion keeps failures assertion-driven.♻️ Suggested test hardening
- attrsStr := rows[0][0].(string) + attrsStr := fmt.Sprint(rows[0][0])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/executor/slow_query_sql_test.go` around lines 582 - 588, The test currently does a direct type assertion rows[0][0].(string) which can panic if the DB driver returns []byte or another type; change to a safe conversion such as using fmt.Sprint(rows[0][0]) (or convert []byte with string(...) after checking type) and assign that to attrsStr before doing require.Contains checks, and add a require.NotNil(rows[0][0]) at the top to keep failures assertion-driven; update imports to include fmt if needed and modify the assertions in the test that reference attrsStr accordingly.pkg/server/conn_test.go (1)
2439-2447: Strengthen the large-payload assertion to prove truncation actually happened.This subtest currently validates only
ConnectAttrsLongestSeen, which can still pass if attribute parsing falls into the warning/ignore path. Add a direct truncation assertion (_truncatedmarker orConnectAttrsLostincrement) to pin behavior.Suggested assertion hardening
err = parse.HandshakeResponseBody(context.Background(), &p, data, offset) require.NoError(t, err) + _, truncated := p.Attrs["_truncated"] + require.True(t, truncated, "expected truncation marker for oversized attrs") + require.Equal(t, int64(1), vardef.ConnectAttrsLost.Load()) + // Attrs are truncated (totalSize=70001 > effectiveLimit=65536, because // limit=-1 maps to effectiveLimit=65536 internally), but LongestSeen // should NOT be updated because totalSize >= 64KB.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/server/conn_test.go` around lines 2439 - 2447, The test only checks vardef.ConnectAttrsLongestSeen but not that attributes were actually truncated; after calling parse.HandshakeResponseBody(context.Background(), &p, data, offset) add an assertion that proves truncation occurred by checking the explicit truncation signal: either assert the parsed payload's truncation marker (p.Attrs._truncated or equivalent field on the parsed attrs) is true or assert vardef.ConnectAttrsLost was incremented as expected (e.g., compared to its prior value), so the subtest verifies both LongestSeen unchanged and that truncation actually happened.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/sessionctx/variable/sysvar.go`:
- Around line 684-693: The sysvar vardef.PerfSchemaSessionConnectAttrsSize lacks
boundary unit tests and SQL integration tests; add unit tests that exercise
SetGlobal/Set logic for values 0 and 65536 (in addition to -1 and small values)
by invoking SetGlobal (which uses TidbOptInt64 and
vardef.ConnectAttrsSize.Store/Load) and asserting the stored value and
truncation/parse behavior, and add integration tests that run SQL like SET
performance_schema_session_connect_attrs_size = <value> and then verify
session-visible behavior (e.g., actual connect attrs size via SELECT or session
variables) to ensure the sysvar plumbing (GetGlobal/SetGlobal) works end-to-end.
---
Nitpick comments:
In `@pkg/executor/slow_query_sql_test.go`:
- Around line 582-588: The test currently does a direct type assertion
rows[0][0].(string) which can panic if the DB driver returns []byte or another
type; change to a safe conversion such as using fmt.Sprint(rows[0][0]) (or
convert []byte with string(...) after checking type) and assign that to attrsStr
before doing require.Contains checks, and add a require.NotNil(rows[0][0]) at
the top to keep failures assertion-driven; update imports to include fmt if
needed and modify the assertions in the test that reference attrsStr
accordingly.
In `@pkg/server/conn_test.go`:
- Around line 2439-2447: The test only checks vardef.ConnectAttrsLongestSeen but
not that attributes were actually truncated; after calling
parse.HandshakeResponseBody(context.Background(), &p, data, offset) add an
assertion that proves truncation occurred by checking the explicit truncation
signal: either assert the parsed payload's truncation marker (p.Attrs._truncated
or equivalent field on the parsed attrs) is true or assert
vardef.ConnectAttrsLost was incremented as expected (e.g., compared to its prior
value), so the subtest verifies both LongestSeen unchanged and that truncation
actually happened.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
pkg/executor/adapter_slow_log.gopkg/executor/slow_query.gopkg/executor/slow_query_sql_test.gopkg/executor/slow_query_test.gopkg/infoschema/tables.gopkg/server/conn_test.gopkg/server/internal/parse/parse.gopkg/sessionctx/vardef/sysvar.gopkg/sessionctx/vardef/tidb_vars.gopkg/sessionctx/variable/noop.gopkg/sessionctx/variable/slow_log.gopkg/sessionctx/variable/statusvar.gopkg/sessionctx/variable/sysvar.gopkg/sessionctx/variable/tests/session_test.go
💤 Files with no reviewable changes (1)
- pkg/sessionctx/variable/noop.go
🚧 Files skipped from review as they are similar to previous changes (8)
- pkg/executor/adapter_slow_log.go
- pkg/executor/slow_query.go
- pkg/sessionctx/variable/tests/session_test.go
- pkg/sessionctx/vardef/tidb_vars.go
- pkg/executor/slow_query_test.go
- pkg/infoschema/tables.go
- pkg/sessionctx/variable/statusvar.go
- pkg/sessionctx/variable/slow_log.go
|
/test all |
|
@jiong-nba: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
jiong-nba
force-pushed
the
feature/slow-log-session-attrs
branch
from
8ed8844 to
a8eea80
Compare
|
Review Complete Findings: 0 issues ℹ️ Learn more details on Pantheon AI. |
jiong-nba
force-pushed
the
feature/slow-log-session-attrs
branch
from
a8eea80 to
2c0b53a
Compare
|
Review Complete Findings: 0 issues ℹ️ Learn more details on Pantheon AI. |
|
/retest |
|
@jiong-nba: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
pkg/server/internal/parse/parse.go
Outdated
| var acceptedSize int64 | ||
| truncated := false | ||
| limit := vardef.ConnectAttrsSize.Load() | ||
| if limit == 0 { |
There was a problem hiding this comment.
Didn't find the rule of this in PM doc, what's the reason for doing this?
|
Review Complete Findings: 1 issues ℹ️ Learn more details on Pantheon AI. |
ti-chi-bot
bot
added
release-note
pkg/sessionctx/vardef/sysvar.go
Outdated
| // PerformanceSchema is the name for 'performance_schema' system variable. | ||
| PerformanceSchema = "performance_schema" | ||
| // PerfSchemaSessionConnectAttrsSize is the name for 'performance_schema_session_connect_attrs_size' system variable. | ||
| PerfSchemaSessionConnectAttrsSize = "performance_schema_session_connect_attrs_size" |
There was a problem hiding this comment.
i think the session_connect_ already have enough info to scope this var, the performance_schema_ part will make this var name too long, have we finalized the design of this varname?
and maybe add max to the name to better reflect its real purpose
There was a problem hiding this comment.
such as session_connect_max_attrs_size
There was a problem hiding this comment.
This feature is designed for compatibility with MySQL, and therefore uses the same system variable names as MySQL.
D3Hunter
left a comment
D3Hunter
left a comment
There was a problem hiding this comment.
AI-generated review based on @D3Hunter's standards; manual review will be performed after all comments are resolved.
Summary
- Total findings: 8
- Inline comments: 8
- Summary-only findings (no inline anchor): 0
Findings (highest risk first)
⚠️ [Major] (4)
_truncatedkey conflates client attributes with TiDB-generated truncation metadata (pkg/server/internal/parse/parse.go:171; pkg/server/internal/parse/parse.go:268)- Slow-log emission of connection attributes lacks a slow-log-specific size budget (
pkg/executor/adapter_slow_log.go:267; pkg/sessionctx/variable/slow_log.go:560; pkg/sessionctx/variable/sysvar.go:684) parseAttrsnow combines protocol decoding, policy enforcement, and global metric mutation (pkg/server/internal/parse/parse.go:184; pkg/server/internal/parse/parse.go:191; pkg/server/internal/parse/parse.go:229; pkg/server/internal/parse/parse.go:245; pkg/server/internal/parse/parse.go:255)- User-visible compatibility changes are introduced without upgrade/runbook guidance (
pkg/infoschema/tables.go:986; pkg/sessionctx/variable/sysvar.go:684; pkg/server/internal/parse/parse.go:139)
🟡 [Minor] (4)
PerfSchema...naming diverges from existingPerformanceSchema...vocabulary (pkg/sessionctx/vardef/sysvar.go:215)parseAttrswarning contract implies singular output but may aggregate multiple warnings (pkg/server/internal/parse/parse.go:184)Session_connect_attrsslow-log fixtures are duplicated across suites (pkg/executor/cluster_table_test.go:400; pkg/infoschema/test/clustertablestest/cluster_tables_test.go:306; pkg/executor/slow_query_sql_test.go:553; pkg/executor/slow_query_test.go:289)- Cross-version and failure-path evidence for new handshake attribute behavior is limited (
pkg/server/internal/parse/parse.go:143; pkg/server/conn_test.go:2287; pkg/server/internal/parse/parse_test.go:49)
| items.StorageKV = stmtCtx.IsTiKV.Load() | ||
| items.StorageMPP = stmtCtx.IsTiFlash.Load() | ||
| items.MemArbitration = stmtCtx.MemTracker.MemArbitration().Seconds() | ||
| if sessVars.ConnectionInfo != nil && len(sessVars.ConnectionInfo.Attributes) > 0 { |
There was a problem hiding this comment.
⚠️ [Major] Slow-log emission of connection attributes lacks a slow-log-specific size budget
Why
Each slow statement now serializes connection attributes, but payload size is bounded only by handshake/session attribute limits, not by a dedicated slow-log emission cap.
Scope
pkg/executor/adapter_slow_log.go:267; pkg/sessionctx/variable/slow_log.go:560; pkg/sessionctx/variable/sysvar.go:684
Risk if unchanged
Under high slow-query volume, per-row payload growth can increase slow-log I/O and downstream parse cost (information_schema.slow_query), worsening tail latency and disk pressure during incidents.
Evidence
SetSlowLogItems attaches sessVars.ConnectionInfo.Attributes to each slow-log item; SlowLogFormat JSON-encodes and writes it when non-empty; performance_schema_session_connect_attrs_size allows up to 65536 bytes (-1 normalized to 64KiB in parse logic).
Change request
Add a slow-log-specific emission cap (or dedicated sysvar) for Session_connect_attrs, and expose truncation/drop observability so operators can bound and monitor log amplification.
There was a problem hiding this comment.
The maximum value of performance_schema_session_connect_attrs_size is 64 KB, making the associated overhead relatively manageable.
There was a problem hiding this comment.
I think in most cases, the connection attribute will not be very large. However, in extreme cases, 64 KB is indeed significantly larger than the size of all current existing columns, and it has little significance to retain. We can confirm with the PM again whether to adjust it, such as adding a switch for control.
|
/retest |
|
@jiong-nba: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
…parsing and tests
|
/retest |
|
@jiong-nba: PRs from untrusted users cannot be marked as trusted with DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@jiong-nba: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: close #66616
Problem Summary:
In a Kubernetes deployment environment, container IPs change frequently. The current slow query log records only IP information, which is insufficient to trace the real source or application of SQL statements. Recording
SESSION_CONNECT_ATTRSin the slow log provides critical client metadata (such asapp_name,_os,_client_name, etc.) injected during the connection handshake.What changed and how does it work?
This PR introduces the feature to inject and log
SESSION_CONNECT_ATTRSinto the slow query log and exposes them through system tables:Slow Query Log Enhancement:
Session_connect_attrsfield in JSON format to the slow query file (pkg/sessionctx/variable/slow_log.go).SESSION_CONNECT_ATTRSJSON column to both theSLOW_QUERYandCLUSTER_SLOW_QUERYsystem tables (pkg/infoschema/tables.go).pkg/executor/slow_query.go).System Variables:
performance_schema_session_connect_attrs_sizefrom anoopvariable to a functionalGLOBALvariable (pkg/sessionctx/variable/sysvar.go), controlling the maximum total byte size of attributes per connection (default: 4096).Status Variables & Truncation Logic:
performance_schema_session_connect_attrs_size(pkg/server/internal/parse/parse.go).GLOBALstatus variables:Performance_schema_session_connect_attrs_longest_seenandPerformance_schema_session_connect_attrs_lostto monitor attribute size and truncation stats._truncatedto record discarded bytes.Check List
Tests
Environment and connection settings
unistore127.0.0.1:4000127.0.0.1:10080/tmp/tidb-slow-attrs-demo.logmysql-connector-python 9.6.0127.0.0.14000rootconn_attrs(normal case):{"app_name":"slowlog_demo","client_case":"attrs_normal"}conn_attrs(truncation case):{"app_name":"slowlog_demo","client_case":"attrs_trunc","extra":"x"*256}Variables used in test
Test SQL
Virtual table query SQL
Observed output: information_schema.slow_query
####Observed output: information_schema.cluster_slow_query
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.
Summary by CodeRabbit
New Features
Tests