Skip to content

feat: add observability metrics for undo log manager#7984

Open
Sumit6307 wants to merge 18 commits into
apache:2.xfrom
Sumit6307:feature/undo-log-observability
Open

feat: add observability metrics for undo log manager#7984
Sumit6307 wants to merge 18 commits into
apache:2.xfrom
Sumit6307:feature/undo-log-observability

Conversation

@Sumit6307
Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR did

Detailed description:
This PR introduces observability metrics to the UndoLogManager in the rm-datasource module. Currently, there is limited visibility into the size and performance of Undo Log operations, which are critical for the stability and performance of AT mode.

The specific changes are:

  1. Added Dependency: Added seata-metrics-core to rm-datasource/pom.xml to enable metrics recording.
  2. Defined Metrics: Added new metric definitions in UndoLogConstants:
    • seata.undo.log.size (Summary): Tracks the size of generated undo logs.
    • seata.undo.log.delete.latency (Timer): Tracks the latency of undo log deletion operations (both single and batch).
    • seata.undo.log.delete.count (Counter): Tracks the number of delete operations.
  3. Instrumentation: Updated AbstractUndoLogManager to record these metrics during flush and delete operations.
  4. Verification: Added UndoLogMetricsTest to verify the metrics recording logic using Mockito.

Ⅱ. Does this pull request fix one issue?

N/A (New Feature / GSoC Contribution)

Ⅲ. Why don't you add test cases (unit test/integration test)?

I have added a new unit test class org.apache.seata.rm.datasource.undo.UndoLogMetricsTest to verify the metrics recording logic independently of the database.

Ⅳ. Describe how to verify it

  1. Run the new unit test: mvn test -pl rm-datasource -Dtest=UndoLogMetricsTest
  2. In a running Seata environment, enable metrics in registry.conf.
  3. Execute AT mode transactions.
  4. Observe the new metrics (seata.undo.log.size, seata.undo.log.delete.latency) in the configured metrics backend (e.g., Prometheus).

Ⅴ. Special notes for reviews

The seata-metrics-core dependency was added to rm-datasource to access RegistryFactory and Id classes.

Sumit6307 and others added 6 commits January 30, 2026 01:27
Copy link
Copy Markdown
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd appreciate it if you could open a related discussion issue and reach consensus there before submitting a PR, rather than coding first and opening a PR immediately.

@Sumit6307
Copy link
Copy Markdown
Contributor Author

I'd appreciate it if you could open a related discussion issue and reach consensus there before submitting a PR, rather than coding first and opening a PR immediately.

@funky-eyes
Okay sir, understood. I am currently working on the PR feat: add observability metrics for undo log manager #7984 .From next time, I’ll make sure to open a related discussion issue first and reach consensus before submitting a PR. Thank you for the guidance.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 5, 2026

Codecov Report

❌ Patch coverage is 72.91667% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.09%. Comparing base (1145e54) to head (37844e3).

Files with missing lines Patch % Lines
...ata/rm/datasource/undo/AbstractUndoLogManager.java 63.88% 7 Missing and 6 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##                2.x    #7984      +/-   ##
============================================
- Coverage     72.12%   72.09%   -0.04%     
  Complexity      872      872              
============================================
  Files          1320     1320              
  Lines         50550    50598      +48     
  Branches       6026     6033       +7     
============================================
+ Hits          36459    36478      +19     
- Misses        11101    11113      +12     
- Partials       2990     3007      +17     
Files with missing lines Coverage Δ
...che/seata/rm/datasource/undo/UndoLogConstants.java 100.00% <100.00%> (ø)
...ata/rm/datasource/undo/AbstractUndoLogManager.java 57.91% <63.88%> (+0.82%) ⬆️

... and 10 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Sumit6307
Copy link
Copy Markdown
Contributor Author

@funky-eyes Please Review and Merge this PR

@Sumit6307 Sumit6307 requested a review from funky-eyes February 5, 2026 14:57
@Sumit6307
Copy link
Copy Markdown
Contributor Author

@funky-eyes Please Check

@Sumit6307
Copy link
Copy Markdown
Contributor Author

@funky-eyes Sir Please Review this PR

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds observability metrics around undo-log generation and deletion in the rm-datasource module, improving visibility into AT-mode undo-log size and delete latency/count.

Changes:

  • Add UndoLog metrics Id definitions (summary/timer/counter) in UndoLogConstants.
  • Instrument undo-log flush and delete (single + batch) paths in AbstractUndoLogManager to record metrics.
  • Add seata-metrics-core dependency and a Mockito-based unit test verifying metric recording.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java Records undo-log size and delete latency/count; adds getRegistry() helper for metrics access
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/UndoLogConstants.java Defines metric Ids for undo-log size and delete timer/counter
rm-datasource/src/test/java/org/apache/seata/rm/datasource/undo/UndoLogMetricsTest.java Adds unit tests verifying metrics are recorded for flush and single delete
rm-datasource/pom.xml Adds seata-metrics-core dependency to enable metrics recording

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rm-datasource/pom.xml
@Sumit6307
Copy link
Copy Markdown
Contributor Author

Hi @funky-eyes ! Just a quick heads-up outside of the PR review: I am currently submitting my GSoC 2026 proposal for Apache Seata, titled "Enhance Seata Go SDK Multi Registry Support and Seata CTL Diagnostic Tool".

I saw FinnTew is listed as a potential mentor. Since you've reviewed most of my recent merged and open PRs, I just wanted to ask if you are also going to be mentoring any GSoC projects this year?

Thanks again for all your reviews on my code so far!

@funky-eyes
Copy link
Copy Markdown
Contributor

Hi @funky-eyes ! Just a quick heads-up outside of the PR review: I am currently submitting my GSoC 2026 proposal for Apache Seata, titled "Enhance Seata Go SDK Multi Registry Support and Seata CTL Diagnostic Tool".

I saw FinnTew is listed as a potential mentor. Since you've reviewed most of my recent merged and open PRs, I just wanted to ask if you are also going to be mentoring any GSoC projects this year?

Thanks again for all your reviews on my code so far!

I'm sorry, but due to personal energy constraints, I will not be mentoring any projects this year, including GSoC and OSPP. As for the project you mentioned, a similar one has already been completed in previous OSPP cycles. You can find it in the historical PRs.

@funky-eyes
Copy link
Copy Markdown
Contributor

Hi @funky-eyes ! Just a quick heads-up outside of the PR review: I am currently submitting my GSoC 2026 proposal for Apache Seata, titled "Enhance Seata Go SDK Multi Registry Support and Seata CTL Diagnostic Tool".

I saw FinnTew is listed as a potential mentor. Since you've reviewed most of my recent merged and open PRs, I just wanted to ask if you are also going to be mentoring any GSoC projects this year?

Thanks again for all your reviews on my code so far!

I'm sorry, but due to personal energy constraints, I will not be mentoring any projects this year, including GSoC and OSPP. As for the project you mentioned, a similar one has already been completed in previous OSPP cycles. You can find it in the historical PRs.

@Sumit6307
Copy link
Copy Markdown
Contributor Author

Sumit6307 commented Apr 1, 2026

Hi @funky-eyes ! Just a quick heads-up outside of the PR review: I am currently submitting my GSoC 2026 proposal for Apache Seata, titled "Enhance Seata Go SDK Multi Registry Support and Seata CTL Diagnostic Tool".
I saw FinnTew is listed as a potential mentor. Since you've reviewed most of my recent merged and open PRs, I just wanted to ask if you are also going to be mentoring any GSoC projects this year?
Thanks again for all your reviews on my code so far!

I'm sorry, but due to personal energy constraints, I will not be mentoring any projects this year, including GSoC and OSPP. As for the project you mentioned, a similar one has already been completed in previous OSPP cycles. You can find it in the historical PRs.

@funky-eyes Hi Sir ,thank you for letting me know, and I completely understand. I really appreciate all the guidance and reviews you've given on my PRs so far.

My GSoC project, titled “Enhance Seata-Go Multi-Registry Support and seata-ctl Diagnostic Tool Capability”, is listed as a GSoC 2026 idea. If you’re comfortable, may I share my proposal with you for a quick review?

Thanks again for all your support

@funky-eyes
Copy link
Copy Markdown
Contributor

Hi @funky-eyes ! Just a quick heads-up outside of the PR review: I am currently submitting my GSoC 2026 proposal for Apache Seata, titled "Enhance Seata Go SDK Multi Registry Support and Seata CTL Diagnostic Tool".
I saw FinnTew is listed as a potential mentor. Since you've reviewed most of my recent merged and open PRs, I just wanted to ask if you are also going to be mentoring any GSoC projects this year?
Thanks again for all your reviews on my code so far!

I'm sorry, but due to personal energy constraints, I will not be mentoring any projects this year, including GSoC and OSPP. As for the project you mentioned, a similar one has already been completed in previous OSPP cycles. You can find it in the historical PRs.

@funky-eyes Hi Sir ,thank you for letting me know, and I completely understand. I really appreciate all the guidance and reviews you've given on my PRs so far.

My GSoC project, titled “Enhance Seata-Go Multi-Registry Support and seata-ctl Diagnostic Tool Capability”, is listed as a GSoC 2026 idea. If you’re comfortable, may I share my proposal with you for a quick review?

Thanks again for all your support

@Code-Fight PTAL

@Sumit6307
Copy link
Copy Markdown
Contributor Author

Hi @funky-eyes,

Thank you for the review and the helpful feedback! I have updated the PR to address all of the comments. Here is a summary of the improvements made:

  1. Optimized Registry Caching: Implemented a Double-Checked Locking (DCL) pattern in AbstractUndoLogManager to cache the Registry instance. This prevents any performance overhead from repeated RegistryFactory.getInstance() calls in the hot path.
  2. Consistent Counter Semantics: Aligned the deletion counter (seata.undo.log.delete.count) to represent "Operation Count" strictly. It now increments exactly by 1 for both single row deletes and batch deletes, ensuring consistent metrics regardless of the operation type.
  3. Dependency Fixes: Added seata-metrics-core and seata-metrics-registry-compact to the pom.xml so the observability metrics record properly at runtime.
  4. Improved Test Coverage: Wrote comprehensive unit tests in UndoLogMetricsTest.java to verify the counter alignments, latency timers, and summary size recordings (including edge cases where 0 rows are affected).
  5. Code Style: Applied mvn spotless:apply to ensure the codebase strictly matches formatting standards.

Please let me know if there are any further adjustments needed or if this is ready to be merged!

@Sumit6307
Copy link
Copy Markdown
Contributor Author

@WangzJi Sir please Check this PR

@Sumit6307
Copy link
Copy Markdown
Contributor Author

@funky-eyes Sir plz Review this PR

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

3 participants