[receiver/oracledb] Add transaction, lock, and recovery metrics - #49067
Conversation
9922b8f to
f17e930
Compare
Surfaces transaction-rollback, lock-timing, recovery, SMON, and RAC cache-fusion statistics already returned by the existing system-statistics query as seven opt-in metrics. No new SQL queries are introduced.
f17e930 to
275a237
Compare
…tion in descriptions - Clarify oracledb.gc.current_block.receive.time is RAC global cache (Cache Fusion), not JVM GC - Note transaction.rollbacks vs user_rollbacks and lock.wait.time vs lock.time distinctions
…sstat statistic) The 'Total Lock Time' statistic does not exist in v$sysstat on supported Oracle versions (confirmed via v$statname on 19c), so the metric never emitted. Background and foreground lock timing remain available via oracledb.lock.time.
# Conflicts: # receiver/oracledbreceiver/internal/metadata/generated_metrics_test.go
…, lean descriptions, simpler tests
…ock-recovery # Conflicts: # receiver/oracledbreceiver/documentation.md # receiver/oracledbreceiver/internal/metadata/config.schema.yaml # receiver/oracledbreceiver/internal/metadata/generated_config.go # receiver/oracledbreceiver/internal/metadata/generated_config_test.go # receiver/oracledbreceiver/internal/metadata/generated_metrics.go # receiver/oracledbreceiver/internal/metadata/generated_metrics_test.go # receiver/oracledbreceiver/internal/metadata/testdata/config.yaml # receiver/oracledbreceiver/metadata.yaml # receiver/oracledbreceiver/scraper.go # receiver/oracledbreceiver/scraper_test.go
Replace oracledb.smon.instance_recovery.posts and oracledb.smon.txn_recovery.posts with a single oracledb.smon.posts metric carrying an oracledb.smon.type attribute (instance|transaction), matching the general-noun + .type attribute convention used elsewhere in the receiver (e.g. oracledb.sort.operations). Also trim the changelog subtext to drop the metric and attribute enumeration.
| oracledb.executions: | ||
| description: The number of times a specific SQL query has been executed (reporting delta). | ||
| type: int | ||
| oracledb.lock.type: |
There was a problem hiding this comment.
oracledb.blocking.lock.type represents actual Oracle lock types, whereas the proposed oracledb.lock.type uses background and foreground. IMHO, lock.type should have a consistent meaning with or without the blocking context. The values background and foreground seem closer to an execution context, such as oracledb.session.type. Could having two different meanings for “lock type” be confusing to users?
There was a problem hiding this comment.
Good catch — you're right that background/foreground are session classes, not lock modes, so overloading lock.type alongside blocking.lock.type was confusing. The two source statistics ("transaction lock background get time" / "foreground wait time") attribute the time by session class. I've dropped oracledb.lock.type and oracledb.lock.time now reuses the existing oracledb.session.type attribute (extended to [background, foreground]), which removes the collision and reuses an attribute the receiver already defines (it's also used by oracledb.db_time).
| component: receiver/oracledb | ||
|
|
||
| # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). | ||
| note: Add transaction, lock, and recovery metrics sourced from the existing v$sysstat query. |
There was a problem hiding this comment.
| note: Add transaction, lock, and recovery metrics sourced from the existing v$sysstat query. | |
| note: Add transaction, lock, and recovery metrics |
Remove implementation details, the subtext provides enough context for the average end user.
| value_type: int | ||
| input_type: string | ||
| unit: "{executions}" | ||
| oracledb.gc.current_block.receive.time: |
There was a problem hiding this comment.
Should receive be some kind of attribute rather than namespace?
There was a problem hiding this comment.
Good point — Oracle also exposes gc current block send time, so receive is really a direction. Renamed the metric to oracledb.gc.current_block.time and added an oracledb.gc.direction attribute (receive now, with send available as a future value), keeping current_block in the name since it's a distinct block state from cr block.
| value_type: int | ||
| input_type: string | ||
| unit: "{queries}" | ||
| oracledb.recovery.blocks_read: |
There was a problem hiding this comment.
"blocks_read" doesn't seem right. Should it be oracledb.recovery.blocks.count and then an attribute for read?
There was a problem hiding this comment.
Agreed, renamed to oracledb.recovery.blocks ({block}) , matching the existing oracledb.redo.blocks. v$sysstat only exposes "recovery blocks read" (no "written" counterpart), so a single-valued read attribute would be permanently decorative — happy to switch to .count + a read attribute if you'd prefer that style.
There was a problem hiding this comment.
Couple comments:
- Sort groups alphabetically
- Remove comments about unit conversions
…tidy descriptions/scraper Reuse the existing oracledb.session.type attribute on oracledb.lock.time (dropping oracledb.lock.type); rename oracledb.gc.current_block.receive.time to oracledb.gc.current_block.time with an oracledb.gc.direction attribute; trim the changelog note, remove unit references from descriptions, and alphabetize the added constant/gate/case groups in scraper.go
Drop the underscore in the leaf name; use the plural-noun count form
| oracledb.executions: | ||
| description: The number of times a specific SQL query has been executed (reporting delta). | ||
| type: int | ||
| oracledb.gc.direction: |
There was a problem hiding this comment.
Why is gc the namespace for this? I see it's used for a gc metric, but since the attribute is not specific to gc maybe it should be named to match its description more closely.
There was a problem hiding this comment.
Good point — the direction isn't gc-specific. Reused the network.io.direction attribute on oracledb.gc.current_block.time instead of a gc-specific one, and generalized that attribute's description to cover both uses.
| value_type: int | ||
| unit: By | ||
| oracledb.transaction.rollbacks: | ||
| description: Number of transactions rolled back. |
There was a problem hiding this comment.
Nit: Maybe add "Total" at the beginning of the description to make it slightly more obvious?
There was a problem hiding this comment.
Done - added 'Total' to the description.
| tableScansRowidRangesStat = "table scans (rowid ranges)" | ||
| userCallsStat = "user calls" | ||
| // Transactions, Locks & Recovery v$sysstat names | ||
| gcCurrentBlockReceiveTime = "gc current block receive time" |
There was a problem hiding this comment.
Please alphabetize this list, as well as the scrape method and added variables in scrape_test.go too.
…acks with Total, alphabetize lists Reuse the existing network.io.direction attribute on oracledb.gc.current_block.time instead of a gc-specific attribute (and generalize that attribute's description). Add 'Total' to the oracledb.transaction.rollbacks description. Merge the added v$sysstat name constants and enable-gate entries into the existing alphabetical lists and alphabetize the enable-config block and switch cases in scraper_test.go.
…ock-recovery # Conflicts: # receiver/oracledbreceiver/internal/metadata/generated_config_test.go # receiver/oracledbreceiver/internal/metadata/generated_metrics.go # receiver/oracledbreceiver/internal/metadata/generated_metrics_test.go # receiver/oracledbreceiver/metadata.yaml
| @@ -393,6 +401,71 @@ func TestScraper_ScrapeOperationalMetrics(t *testing.T) { | |||
| } | |||
|
|
|||
| // scrapeWithConfig starts a scraper over the shared fake responses and returns one scrape. | |||
There was a problem hiding this comment.
This comment needs to be moved to be next to its relevant method
|
This PR has review comments. Review suggestions, whether from maintainers or automated reviewers, aren't always correct or required. Please evaluate each comment on its merits, then make sure each thread has a clear outcome. For example, link to the commit if you applied a suggestion, explain why it wasn't applied, or ask a follow-up question. Automation flags a PR for human review once every review thread has a reply or is marked as resolved. Status across open PRs is visible on the pull request dashboard. |
…ock-recovery # Conflicts: # receiver/oracledbreceiver/documentation.md # receiver/oracledbreceiver/internal/metadata/generated_config_test.go # receiver/oracledbreceiver/scraper_test.go
…ock-recovery # Conflicts: # receiver/oracledbreceiver/documentation.md # receiver/oracledbreceiver/internal/metadata/config.schema.yaml # receiver/oracledbreceiver/internal/metadata/generated_config.go # receiver/oracledbreceiver/internal/metadata/generated_config_test.go # receiver/oracledbreceiver/internal/metadata/generated_metrics.go # receiver/oracledbreceiver/internal/metadata/generated_metrics_test.go # receiver/oracledbreceiver/internal/metadata/testdata/config.yaml # receiver/oracledbreceiver/metadata.yaml
|
Done — conflicts resolved. |
Description
The Oracle DB receiver scrapes
v$sysstatwithSELECT * FROM v$sysstat, which returns all system-statistics rows. However, the scraper only processes a subset of those rows in itsswitchblock and silently discards the rest — including the transaction-rollback, lock-contention, and recovery statistics. This change surfaces 7 discardedv$sysstatstatistics as 5 new opt-in metrics, giving operators visibility into commit/rollback churn, enqueue/lock pressure, instance- and transaction-recovery activity, and RAC cache-fusion latency — core Oracle durability and availability diagnostics.This change adds 5 new opt-in metrics (disabled by default, stability: development):
oracledb.transaction.rollbacks— Number of transactions rolled back. No attributes. Coversv$sysstat:transaction rollbacks. Distinct from the existingoracledb.user_rollbacks(which counts only user-issuedROLLBACKstatements); this counts all transaction rollbacks, including internal/recursive ones.oracledb.lock.time— Cumulative time, in seconds, spent on transaction lock activity. Attribute:oracledb.session.type(background|foreground). Coversv$sysstat:transaction lock background get time,transaction lock foreground wait time(the raw centisecond value is divided by 100 in the scraper).oracledb.recovery.blocks— Number of blocks read during instance or media recovery. No attributes. Coversv$sysstat:recovery blocks read.oracledb.smon.posts— Number of times SMON was posted to perform recovery. Attribute:oracledb.smon.type(instance|transaction). Coversv$sysstat:SMON posted for instance recovery(instance),SMON posted for txn recovery for other instances(transaction, RAC cross-instance).oracledb.gc.current_block.time— Cumulative time, in seconds, spent transferring current blocks between instances over Oracle RAC cache fusion. Attribute:network.io.direction(receive). Coversv$sysstat:gc current block receive time(cs ÷ 100). Here "gc" denotes Oracle's global cache (Cache Fusion), not JVM garbage collection; the value is 0 on a single-instance (non-RAC) database.oracledb.lock.timeandoracledb.gc.current_block.timeare emitted as aSumwithaggregation_temporality: cumulative,monotonic: true, value typedouble, units. The remaining three are emitted asSumwithaggregation_temporality: cumulative,monotonic: true, value typeint. Per-metric units:s(oracledb.lock.time,oracledb.gc.current_block.time);{rollback}(oracledb.transaction.rollbacks);{block}(oracledb.recovery.blocks);{post}(oracledb.smon.posts).The two time metrics report seconds (
s) rather than the rawcs(centisecond) units that Oracle exposes viav$sysstat; the scraper divides the raw value by 100 before recording, matching the conversion already in place for the existingoracledb.cpu_timemetric.These metrics can be enabled in the collector configuration:
Link to tracking issue
Fixes #49064
Testing
Unit tests added in
scraper_test.go:TestScraper_ScrapeTransactionLockRecoveryMetricsexercises all 5 new metrics end-to-end through the scraper using the existing fake DB client, asserting one expected value per data point (2oracledb.lock.timedata points — one peroracledb.session.type— plus 2oracledb.smon.postsdata points — one peroracledb.smon.type— plus 3 standalone data points = 7 data points per scrape across the 5 metrics).queryResponses[statsSQL]fixture is extended with 7 new fakev$sysstatrows (one per covered NAME), soTestScraper_Scrape,TestScraper_ScrapeOperationalMetrics, andTestScraper_ScrapeIOPerformanceMetricscontinue to pass unchanged.transaction lock background get time/transaction lock foreground wait timeproduce 3.5/12.0 s on the background/foreground data points; 640 cs fromgc current block receive timeproduces 6.4 s.internal/metadata/generated_metrics_test.goandgenerated_config_test.goare regenerated bymake generateand cover the new metric configs / metric builders.Documentation
Auto-generated
documentation.mdupdated with descriptions and metadata for the 5 new metrics, the neworacledb.smon.typeattribute, the reusednetwork.io.directionattribute (onoracledb.gc.current_block.time), and the extendedoracledb.session.typeattribute (nowbackground/foreground).internal/metadata/generated_*.goandinternal/metadata/testdata/config.yamlregenerated viamake generate.internal/metadata/config.schema.yamlwas updated for the 5 new metric stanzas, as that file is not produced bymake generate.Authorship