-
Notifications
You must be signed in to change notification settings - Fork 3.8k
oracledbreceiver: optimize query sample collection by splitting into two-pass approach to avoid full cursor cache scan #49875
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
1edf3dc
ef31e02
29d3717
5c3de48
abe9939
74113f2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Use this changelog template to create an entry for release notes. | ||
|
|
||
| # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
| change_type: enhancement | ||
|
|
||
| # The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog) | ||
| component: receiver/oracledb | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Speed up query-sample collection by splitting it into two queries so the SQL text/plan lookup no longer joins V$SESSION against the entire cursor cache. | ||
|
|
||
| # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. | ||
| issues: [49874] | ||
|
|
||
| # (Optional) One or more lines of additional information to render under the primary note. | ||
| # These lines will be padded with 2 spaces and then inserted directly into the document. | ||
| # Use pipe (|) for multiline entries. | ||
| subtext: | | ||
| The session query no longer joins V$SQL (which forced Oracle to scan the whole | ||
| shared-pool cursor cache and materialize SQL_FULLTEXT for every cursor). Instead, | ||
| the receiver collects the active sessions first, then fetches SQL_FULLTEXT / | ||
| CHILD_ADDRESS / PLAN_HASH_VALUE from V$SQL for only those sql_ids via | ||
| WHERE SQL_ID IN (...), and joins the results in the collector. Sessions whose | ||
| cursor has aged out of the shared pool are skipped, preserving the previous | ||
| inner-join semantics. | ||
|
|
||
| # If your change doesn't affect end users or the exported elements of any package, | ||
| # you should instead start your pull request title with [chore] or use the "Skip Changelog" label. | ||
| # Optional: The change log or logs in which this entry should be included. | ||
| # e.g. '[user]' or '[user, api]' | ||
| # Include 'user' if the change is relevant to end users. | ||
| # Include 'api' if there is a change to a library API. | ||
| # Default: '[user]' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import ( | |
| "encoding/json" | ||
| "errors" | ||
| "log" | ||
| "maps" | ||
| "os" | ||
| "path/filepath" | ||
| "sort" | ||
|
|
@@ -834,41 +835,67 @@ func TestScraper_ScrapeTopNLogs(t *testing.T) { | |
|
|
||
| var samplesQueryResponses = map[string][]metricRow{ | ||
| samplesQuery: {{ | ||
| "ACTION": "00-0af7651916cd43dd8448eb211c80319c-a7ad6b7169203331-01", "MACHINE": "TEST-MACHINE", "USERNAME": "ADMIN", "SCHEMANAME": "ADMIN", "SQL_ID": "48bc50b6fuz4y", "WAIT_CLASS": "ONE", "WAIT_TIME_SEC": "0.5", "PROCEDURE_NAME": "BLAH", "CHILD_ADDRESS": "SDF3SDF1234D", | ||
| "SQL_CHILD_NUMBER": "0", "SID": "675", "SERIAL#": "51295", "SQL_FULLTEXT": "test_query", "OSUSER": "test-user", "PROCESS": "1115", "PROCEDURE_TYPE": "PROCEDURE_TYPE-A", "PROCEDURE_ID": "12345", | ||
| "PORT": "54440", "PROGRAM": "Oracle SQL Developer for VS Code", "MODULE": "Oracle SQL Developer for VS Code", "STATUS": "ACTIVE", "STATE": "WAITED KNOWN TIME", "PLAN_HASH_VALUE": "4199919568", "DURATION_SEC": "1", "SERVICE_NAME": "", "DB_NAMESPACE": "", | ||
| "ACTION": "00-0af7651916cd43dd8448eb211c80319c-a7ad6b7169203331-01", "MACHINE": "TEST-MACHINE", "USERNAME": "ADMIN", "SCHEMANAME": "ADMIN", "SQL_ID": "48bc50b6fuz4y", "WAIT_CLASS": "ONE", "WAIT_TIME_SEC": "0.5", "PROCEDURE_NAME": "BLAH", | ||
| "SQL_CHILD_NUMBER": "0", "LOOKUP_SQL_ID": "48bc50b6fuz4y", "LOOKUP_CHILD_NUMBER": "0", "SID": "675", "SERIAL#": "51295", "OSUSER": "test-user", "PROCESS": "1115", "PROCEDURE_TYPE": "PROCEDURE_TYPE-A", "PROCEDURE_ID": "12345", | ||
| "PORT": "54440", "PROGRAM": "Oracle SQL Developer for VS Code", "MODULE": "Oracle SQL Developer for VS Code", "STATUS": "ACTIVE", "STATE": "WAITED KNOWN TIME", "DURATION_SEC": "1", "SERVICE_NAME": "", "DB_NAMESPACE": "", | ||
| "SQL_EXEC_START": "2026-01-01T12:00:00Z", "LOGON_TIME": "2026-01-01T12:00:00Z", "SESSION_DURATION_SEC": "0", | ||
| "BLOCKING_SESSION": "", "FINAL_BLOCKING_SESSION": "", "BLOCKING_SESSION_STATUS": "", "SECONDS_IN_WAIT": "0", | ||
| "BLOCKING_START_TIME": "", "LOCK_TYPE": "", "LOCK_MODE": "", "BLOCKED_OBJECT_OWNER": "", "BLOCKED_OBJECT_NAME": "", | ||
| }}, | ||
| "invalidQuery": {{ | ||
| "MACHINE": "TEST-MACHINE", "USERNAME": "ADMIN", "SCHEMANAME": "ADMIN", "SQL_ID": "48bc50b6fuz4y", | ||
| "SQL_CHILD_NUMBER": "0", "S.SID": "675", "SERIAL#": "51295", "SQL_FULLTEXT": "test_query", "OSUSER": "test-user", "PROCESS": "1115", | ||
| "PORT": "54440", "PROGRAM": "Oracle SQL Developer for VS Code", "MODULE": "Oracle SQL Developer for VS Code", "STATUS": "ACTIVE", "STATE": "WAITED KNOWN TIME", "PLAN_HASH_VALUE": "4199919568", "DURATION_SEC": "", | ||
| "SQL_CHILD_NUMBER": "0", "LOOKUP_SQL_ID": "48bc50b6fuz4y", "LOOKUP_CHILD_NUMBER": "0", "S.SID": "675", "SERIAL#": "51295", "OSUSER": "test-user", "PROCESS": "1115", | ||
| "PORT": "54440", "PROGRAM": "Oracle SQL Developer for VS Code", "MODULE": "Oracle SQL Developer for VS Code", "STATUS": "ACTIVE", "STATE": "WAITED KNOWN TIME", "DURATION_SEC": "", | ||
| "SQL_EXEC_START": "", "LOGON_TIME": "", "SESSION_DURATION_SEC": "0", | ||
| "BLOCKING_SESSION": "", "FINAL_BLOCKING_SESSION": "", "BLOCKING_SESSION_STATUS": "", "SECONDS_IN_WAIT": "0", | ||
| "BLOCKING_START_TIME": "", "LOCK_TYPE": "", "LOCK_MODE": "", "BLOCKED_OBJECT_OWNER": "", "BLOCKED_OBJECT_NAME": "", | ||
| }}, | ||
| "blockedQuery": {{ | ||
| // Blocked session waiting on SID 100 | ||
| "ACTION": "", "MACHINE": "DB-CLIENT-HOST", "USERNAME": "APP_USER", "SCHEMANAME": "APP_USER", "SQL_ID": "9fkq2mxyzabc1", "WAIT_CLASS": "Application", "PROCEDURE_NAME": "", "CHILD_ADDRESS": "ABCD1234", | ||
| "SQL_CHILD_NUMBER": "0", "SID": "200", "SERIAL#": "12345", "SQL_FULLTEXT": "UPDATE orders SET status = 1 WHERE id = 42", "OSUSER": "oracle", "PROCESS": "9876", "PROCEDURE_TYPE": "", "PROCEDURE_ID": "", | ||
| "PORT": "54441", "PROGRAM": "JDBC Thin Client", "MODULE": "app", "STATUS": "ACTIVE", "STATE": "WAITING", "PLAN_HASH_VALUE": "1234567890", "DURATION_SEC": "15", "SERVICE_NAME": "ORCL", "DB_NAMESPACE": "ORCLPDB1", | ||
| "ACTION": "", "MACHINE": "DB-CLIENT-HOST", "USERNAME": "APP_USER", "SCHEMANAME": "APP_USER", "SQL_ID": "9fkq2mxyzabc1", "WAIT_CLASS": "Application", "PROCEDURE_NAME": "", | ||
| "SQL_CHILD_NUMBER": "0", "LOOKUP_SQL_ID": "9fkq2mxyzabc1", "LOOKUP_CHILD_NUMBER": "0", "SID": "200", "SERIAL#": "12345", "OSUSER": "oracle", "PROCESS": "9876", "PROCEDURE_TYPE": "", "PROCEDURE_ID": "", | ||
| "PORT": "54441", "PROGRAM": "JDBC Thin Client", "MODULE": "app", "STATUS": "ACTIVE", "STATE": "WAITING", "DURATION_SEC": "15", "SERVICE_NAME": "ORCL", "DB_NAMESPACE": "ORCLPDB1", | ||
| "SQL_EXEC_START": "2026-05-06T09:59:45Z", "LOGON_TIME": "2026-05-06T09:00:00Z", "SESSION_DURATION_SEC": "3600", | ||
| "BLOCKING_SESSION": "100", "FINAL_BLOCKING_SESSION": "100", "BLOCKING_SESSION_STATUS": "VALID", "SECONDS_IN_WAIT": "15", | ||
| "BLOCKING_START_TIME": "2026-05-06T10:00:00Z", "LOCK_TYPE": "TX", "LOCK_MODE": "EXCLUSIVE", "BLOCKED_OBJECT_OWNER": "APP_USER", "BLOCKED_OBJECT_NAME": "ORDERS", | ||
| }}, | ||
| "idleBlockerQuery": {{ | ||
| // Idle session (blocker) that is holding a lock — no longer ACTIVE but appearing in BLOCKING_SESSION subquery | ||
| "ACTION": "", "MACHINE": "DBA-WORKSTATION", "USERNAME": "DBA_USER", "SCHEMANAME": "DBA_USER", "SQL_ID": "7abc123def456", "WAIT_CLASS": "", "PROCEDURE_NAME": "", "CHILD_ADDRESS": "DEADBEEF", | ||
| "SQL_CHILD_NUMBER": "0", "SID": "100", "SERIAL#": "5678", "SQL_FULLTEXT": "UPDATE orders SET status = 2 WHERE id = 42", "OSUSER": "dba", "PROCESS": "1234", "PROCEDURE_TYPE": "", "PROCEDURE_ID": "", | ||
| "PORT": "54442", "PROGRAM": "SQL*Plus", "MODULE": "", "STATUS": "INACTIVE", "STATE": "WAITED KNOWN TIME", "PLAN_HASH_VALUE": "9876543210", "DURATION_SEC": "120", "SERVICE_NAME": "ORCL", "DB_NAMESPACE": "ORCLPDB1", | ||
| "ACTION": "", "MACHINE": "DBA-WORKSTATION", "USERNAME": "DBA_USER", "SCHEMANAME": "DBA_USER", "SQL_ID": "7abc123def456", "WAIT_CLASS": "", "PROCEDURE_NAME": "", | ||
| "SQL_CHILD_NUMBER": "0", "LOOKUP_SQL_ID": "7abc123def456", "LOOKUP_CHILD_NUMBER": "0", "SID": "100", "SERIAL#": "5678", "OSUSER": "dba", "PROCESS": "1234", "PROCEDURE_TYPE": "", "PROCEDURE_ID": "", | ||
| "PORT": "54442", "PROGRAM": "SQL*Plus", "MODULE": "", "STATUS": "INACTIVE", "STATE": "WAITED KNOWN TIME", "DURATION_SEC": "120", "SERVICE_NAME": "ORCL", "DB_NAMESPACE": "ORCLPDB1", | ||
| "SQL_EXEC_START": "", "LOGON_TIME": "", "SESSION_DURATION_SEC": "0", | ||
| "BLOCKING_SESSION": "", "FINAL_BLOCKING_SESSION": "", "BLOCKING_SESSION_STATUS": "", "SECONDS_IN_WAIT": "0", | ||
| "BLOCKING_START_TIME": "", "LOCK_TYPE": "", "LOCK_MODE": "", "BLOCKED_OBJECT_OWNER": "", "BLOCKED_OBJECT_NAME": "", | ||
| }}, | ||
| } | ||
|
|
||
| var sampleStatsRows = []metricRow{ | ||
| {"SQL_ID": "48bc50b6fuz4y", "CHILD_NUMBER": "0", "CHILD_ADDRESS": "SDF3SDF1234D", "PLAN_HASH_VALUE": "4199919568", "SQL_FULLTEXT": "test_query"}, | ||
| {"SQL_ID": "9fkq2mxyzabc1", "CHILD_NUMBER": "0", "CHILD_ADDRESS": "ABCD1234", "PLAN_HASH_VALUE": "1234567890", "SQL_FULLTEXT": "UPDATE orders SET status = 1 WHERE id = 42"}, | ||
| {"SQL_ID": "7abc123def456", "CHILD_NUMBER": "0", "CHILD_ADDRESS": "DEADBEEF", "PLAN_HASH_VALUE": "9876543210", "SQL_FULLTEXT": "UPDATE orders SET status = 2 WHERE id = 42"}, | ||
| } | ||
|
|
||
| // copy so subtests don't share mutated fixture rows | ||
| func cloneRows(rows []metricRow) []metricRow { | ||
| out := make([]metricRow, len(rows)) | ||
| for i, r := range rows { | ||
| nr := make(metricRow, len(r)) | ||
| maps.Copy(nr, r) | ||
| out[i] = nr | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| func twoPassSampleClient(sessionRows []metricRow) func(*sql.DB, string, *zap.Logger) dbClient { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These tests route through Could you add a test that drives >1000 distinct sql_ids and asserts on the generated SQL + args? Something like: On the current code this fails at batch 2 (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a test case to cover this scenerio |
||
| return func(_ *sql.DB, s string, _ *zap.Logger) dbClient { | ||
| if strings.Contains(s, "FROM V$SQL") && strings.Contains(s, "SQL_ID IN") { | ||
| return &fakeDbClient{Responses: [][]metricRow{cloneRows(sampleStatsRows)}} | ||
| } | ||
| return &fakeDbClient{Responses: [][]metricRow{cloneRows(sessionRows)}} | ||
| } | ||
| } | ||
|
|
||
| var sessionEventQueryResponses = map[string][]metricRow{ | ||
| sessionEventQuery: { | ||
| { | ||
|
|
@@ -903,42 +930,24 @@ func TestSamplesQuery(t *testing.T) { | |
| checkBlockingAttr bool | ||
| }{ | ||
| { | ||
| name: "valid", | ||
| dbclientFn: func(_ *sql.DB, _ string, _ *zap.Logger) dbClient { | ||
| return &fakeDbClient{ | ||
| Responses: [][]metricRow{ | ||
| samplesQueryResponses[samplesQuery], | ||
| }, | ||
| } | ||
| }, | ||
| name: "valid", | ||
| dbclientFn: twoPassSampleClient(samplesQueryResponses[samplesQuery]), | ||
| goldenFile: filepath.Join("testdata", "expectedSamplesFile.yaml"), | ||
| }, | ||
| { | ||
| name: "bad samples data", | ||
| dbclientFn: func(_ *sql.DB, _ string, _ *zap.Logger) dbClient { | ||
| return &fakeDbClient{Responses: [][]metricRow{ | ||
| samplesQueryResponses["invalidQuery"], | ||
| }} | ||
| }, | ||
| errWanted: `failed to parse int64 for Duration, value was : strconv.ParseFloat: parsing "": invalid syntax`, | ||
| name: "bad samples data", | ||
| dbclientFn: twoPassSampleClient(samplesQueryResponses["invalidQuery"]), | ||
| errWanted: `failed to parse int64 for Duration, value was : strconv.ParseFloat: parsing "": invalid syntax`, | ||
| }, | ||
| { | ||
| name: "blocked session emits blocking attributes", | ||
| dbclientFn: func(_ *sql.DB, _ string, _ *zap.Logger) dbClient { | ||
| return &fakeDbClient{Responses: [][]metricRow{ | ||
| samplesQueryResponses["blockedQuery"], | ||
| }} | ||
| }, | ||
| name: "blocked session emits blocking attributes", | ||
| dbclientFn: twoPassSampleClient(samplesQueryResponses["blockedQuery"]), | ||
| goldenFile: filepath.Join("testdata", "expectedBlockedSessionFile.yaml"), | ||
| checkBlockingAttr: true, | ||
| }, | ||
| { | ||
| name: "idle blocker session emits empty blocking attributes", | ||
| dbclientFn: func(_ *sql.DB, _ string, _ *zap.Logger) dbClient { | ||
| return &fakeDbClient{Responses: [][]metricRow{ | ||
| samplesQueryResponses["idleBlockerQuery"], | ||
| }} | ||
| }, | ||
| name: "idle blocker session emits empty blocking attributes", | ||
| dbclientFn: twoPassSampleClient(samplesQueryResponses["idleBlockerQuery"]), | ||
| goldenFile: filepath.Join("testdata", "expectedIdleBlockerFile.yaml"), | ||
| }, | ||
| } | ||
|
|
@@ -1006,6 +1015,47 @@ func TestSamplesQuery(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| func TestSamplesQueryCursorAgedOut(t *testing.T) { | ||
| logsCfg := metadata.DefaultLogsBuilderConfig() | ||
| logsCfg.ResourceAttributes.OracledbInstanceName.Enabled = true | ||
| logsCfg.ResourceAttributes.HostName.Enabled = true | ||
| logsCfg.Events.DbServerTopQuery.Enabled = false | ||
| logsCfg.Events.DbServerQuerySample.Enabled = true | ||
|
|
||
| scrpr := oracleScraper{ | ||
| logger: zap.NewNop(), | ||
| dbProviderFunc: func() (*sql.DB, error) { return nil, nil }, | ||
| clientProviderFunc: func(_ *sql.DB, s string, _ *zap.Logger) dbClient { | ||
| if strings.Contains(s, "FROM V$SQL") && strings.Contains(s, "SQL_ID IN") { | ||
| // Cursor aged out of the shared pool: no V$SQL row for the session's sql_id. | ||
| return &fakeDbClient{Responses: [][]metricRow{{}}} | ||
| } | ||
| return &fakeDbClient{Responses: [][]metricRow{cloneRows(samplesQueryResponses[samplesQuery])}} | ||
| }, | ||
| id: component.ID{}, | ||
| lb: metadata.NewLogsBuilder(logsCfg, receivertest.NewNopSettings(metadata.Type)), | ||
| logsBuilderConfig: logsCfg, | ||
| obfuscator: newObfuscator(), | ||
| instanceName: "oraclehost:1521/ORCL", | ||
| serviceInstanceID: getInstanceID("oraclehost:1521/ORCL", zap.NewNop()), | ||
| } | ||
| require.NoError(t, scrpr.start(t.Context(), componenttest.NewNopHost())) | ||
| defer func() { assert.NoError(t, scrpr.shutdown(t.Context())) }() | ||
|
|
||
| logs, err := scrpr.scrapeLogs(t.Context()) | ||
| require.NoError(t, err) | ||
|
|
||
| // No query_sample records should be emitted when the cursor is gone. | ||
| recordCount := 0 | ||
| for i := 0; i < logs.ResourceLogs().Len(); i++ { | ||
| sls := logs.ResourceLogs().At(i).ScopeLogs() | ||
| for j := 0; j < sls.Len(); j++ { | ||
| recordCount += sls.At(j).LogRecords().Len() | ||
| } | ||
| } | ||
| assert.Zero(t, recordCount, "session whose cursor aged out of V$SQL must not emit a query_sample") | ||
| } | ||
|
|
||
| func TestScraperWithQueryComments(t *testing.T) { | ||
| sql := "/* application=test-123 */ SELECT * FROM test_table" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| /* otel-collector */ SELECT | ||
| SQL_ID, | ||
| CHILD_NUMBER, | ||
| RAWTOHEX(CHILD_ADDRESS) AS CHILD_ADDRESS, | ||
| PLAN_HASH_VALUE, | ||
| SQL_FULLTEXT | ||
| FROM V$SQL | ||
| WHERE SQL_ID IN (%s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reuses a global
:1..:Nplaceholder list across batches, but each iteration is a separate statement whose args are bound at positions 1..len(batchIDs). Batch 1 is fine (:1..:1000); batch 2 slices out:1001..:2000yet still passes only 1000 args, so those bind positions are never supplied. Depending on how go-ora maps numbered placeholders that's either a silent wrong-bind orORA-01008: not all variables are bound.Simplest robust fix is to regenerate
:1..:ninside the batch loop so the numbers always match the args for that call (same convention asgetChildAddressToPlanMap). Replace the id-collection + batch loop with something like:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this. Fixed in the latest commit