You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "Sumit6307 (via GitHub)" <gi...@apache.org> on 2026/02/02 12:17:07 UTC
[PR] feat: add observability metrics for undo log manager [incubator-seata]
Sumit6307 opened a new pull request, #7984:
URL: https://github.com/apache/incubator-seata/pull/7984
### Ⅰ. Describe what this PR did
Detailed description:
This PR introduces observability metrics to the [UndoLogManager](cci:2://file:///c:/Users/Sumit/OneDrive/Desktop/GSOC_2026/gsoc_incubator-seata/gsoc_incubator-seata/rm-datasource/src/test/java/org/apache/seata/rm/datasource/undo/UndoLogManagerTest.java:36:0-87:1) 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](cci:7://file:///c:/Users/Sumit/OneDrive/Desktop/GSOC_2026/gsoc_incubator-seata/gsoc_incubator-seata/rm-datasource/pom.xml:0:0-0:0) to enable metrics recording.
2. **Defined Metrics:** Added new metric definitions in [UndoLogConstants](cci:2://file:///c:/Users/Sumit/OneDrive/Desktop/GSOC_2026/gsoc_incubator-seata/gsoc_incubator-seata/rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/UndoLogConstants.java:25:0-57:1):
* `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](cci:2://file:///c:/Users/Sumit/OneDrive/Desktop/GSOC_2026/gsoc_incubator-seata/gsoc_incubator-seata/rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:60:0-616:1) to record these metrics during flush and delete operations.
4. **Verification:** Added [UndoLogMetricsTest](cci:2://file:///c:/Users/Sumit/OneDrive/Desktop/GSOC_2026/gsoc_incubator-seata/gsoc_incubator-seata/rm-datasource/src/test/java/org/apache/seata/rm/datasource/undo/UndoLogMetricsTest.java:28:0-86:1) to verify the metrics recording logic using Mockito.
### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->
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](cci:2://file:///c:/Users/Sumit/OneDrive/Desktop/GSOC_2026/gsoc_incubator-seata/gsoc_incubator-seata/metrics/seata-metrics-core/src/main/java/org/apache/seata/metrics/registry/RegistryFactory.java:32:0-50:1) and `Id` classes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4174273618
> > > 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
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4283976607
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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-3854107835
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `80.55556%` with `7 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 71.77%. Comparing base ([`3594589`](https://app.codecov.io/gh/apache/incubator-seata/commit/35945899a2134dd05f9488c34386be65aca26f3b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3b90394`](https://app.codecov.io/gh/apache/incubator-seata/commit/3b903948d5079073fa0f79592add7ffc1d88ced2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ata/rm/datasource/undo/AbstractUndoLogManager.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fundo%2FAbstractUndoLogManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS91bmRvL0Fic3RyYWN0VW5kb0xvZ01hbmFnZXIuamF2YQ==) | 70.83% | [6 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 2.x #7984 +/- ##
============================================
+ Coverage 71.75% 71.77% +0.01%
Complexity 872 872
============================================
Files 1300 1300
Lines 49721 49755 +34
Branches 5908 5912 +4
============================================
+ Hits 35679 35713 +34
- Misses 11099 11103 +4
+ Partials 2943 2939 -4
```
| [Files with missing lines](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...che/seata/rm/datasource/undo/UndoLogConstants.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fundo%2FUndoLogConstants.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS91bmRvL1VuZG9Mb2dDb25zdGFudHMuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...ata/rm/datasource/undo/AbstractUndoLogManager.java](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?src=pr&el=tree&filepath=rm-datasource%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fseata%2Frm%2Fdatasource%2Fundo%2FAbstractUndoLogManager.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2VhdGEvcm0vZGF0YXNvdXJjZS91bmRvL0Fic3RyYWN0VW5kb0xvZ01hbmFnZXIuamF2YQ==) | `57.95% <70.83%> (+0.86%)` | :arrow_up: |
... and [11 files with indirect coverage changes](https://app.codecov.io/gh/apache/incubator-seata/pull/7984/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
[](https://app.codecov.io/gh/apache/incubator-seata/pull/7984?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4007426453
@funky-eyes Please Check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-3854194200
@funky-eyes Please Review and Merge this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4129094219
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!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4168026104
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4322310959
@funky-eyes Sir plz Review this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "funky-eyes (via GitHub)" <gi...@apache.org>.
funky-eyes commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4168016893
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4168744663
> > 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.
Got it! We’ll remove any mention of “previous OSPP work” and just focus on your project and asking for feedback. Here’s a clean version:
---
@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, which is why I chose it. I would be grateful if I could share my proposal with you for a quick review. Even brief feedback would help me refine it further.
Thanks again for all your support!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4041058705
@funky-eyes Sir Please Review this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-4296715256
@WangzJi Sir please Check this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Sumit6307 (via GitHub)" <gi...@apache.org>.
Sumit6307 commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-3839298918
> 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "WangzJi (via GitHub)" <gi...@apache.org>.
WangzJi commented on PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#issuecomment-5056026680
Thanks for the contribution, @Sumit6307! Instrumenting the undo log manager is a good direction, but I don't think the current metrics let us locate where the slowness is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] feat: add observability metrics for undo log manager [incubator-seata]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7984:
URL: https://github.com/apache/incubator-seata/pull/7984#discussion_r2945610351
##########
rm-datasource/pom.xml:
##########
@@ -168,5 +168,10 @@
<artifactId>json-common</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
+ <groupId>${project.groupId}</groupId>
+ <artifactId>seata-metrics-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
Review Comment:
Only `seata-metrics-core` is added, but the default metrics registry type is `compact` (see `DefaultValues.DEFAULT_METRICS_REGISTRY_TYPE`), whose implementation lives in `seata-metrics-registry-compact`. Without also including a registry implementation (or explicitly disabling metrics by default for RM), `RegistryFactory.getInstance()` will fail to load a provider and metrics will never record (and may cause repeated warning logs). Add the appropriate registry dependency (or switch to `seata-metrics-all`) and/or ensure metrics are disabled unless the registry/exporter is present.
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -190,6 +205,16 @@ public void batchDeleteUndoLog(Set<String> xids, Set<Long> branchIds, Connection
e = new SQLException(e);
}
throw (SQLException) e;
+ } finally {
+ Registry registry = getRegistry();
+ if (registry != null) {
+ registry.getTimer(UndoLogConstants.TIMER_UNDO_LOG_DELETE_LATENCY)
+ .record(System.nanoTime() - start, TimeUnit.NANOSECONDS);
+ if (totalDeleteRows > 0) {
+ registry.getCounter(UndoLogConstants.COUNTER_UNDO_LOG_DELETE_COUNT)
+ .increase(totalDeleteRows);
Review Comment:
In `batchDeleteUndoLog`, the delete counter is increased by `totalDeleteRows` (rows affected by the main delete), while `deleteUndoLog` always increases by 1. This makes the `COUNTER_UNDO_LOG_DELETE_COUNT` metric semantics inconsistent (operation count vs row count) and also ignores rows deleted by `deleteSubPST`. Align the counter to a single meaning (e.g., increment by 1 per delete call, or track deleted-row counts separately and include both statements).
##########
rm-datasource/src/main/java/org/apache/seata/rm/datasource/undo/AbstractUndoLogManager.java:
##########
@@ -586,4 +615,19 @@ public boolean hasUndoLogTable(Connection conn) {
protected String getCheckUndoLogTableExistSql() {
return CHECK_UNDO_LOG_TABLE_EXIST_SQL;
}
+
+ /**
+ * Get the metrics registry instance.
+ * This method is protected to allow testing with Mockito spy.
+ *
+ * @return the Registry instance, or null if metrics are not enabled
+ */
+ protected Registry getRegistry() {
+ try {
+ return RegistryFactory.getInstance();
+ } catch (Throwable t) {
+ LOGGER.warn("Failed to get metrics registry: {}", t.getMessage());
+ return null;
+ }
Review Comment:
`getRegistry()` calls `RegistryFactory.getInstance()` on every undo-log flush/delete, which repeatedly hits config + service-loading and can become a hot-path overhead. Consider caching the resolved `Registry` (including a cached "unavailable" state) and avoid logging on every call; also log the exception with stack trace (current code logs only `t.getMessage()`, losing diagnosis).
##########
rm-datasource/src/test/java/org/apache/seata/rm/datasource/undo/UndoLogMetricsTest.java:
##########
@@ -0,0 +1,100 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.rm.datasource.undo;
+
+import org.apache.seata.metrics.Id;
+import org.apache.seata.metrics.registry.Registry;
+import org.apache.seata.rm.datasource.ConnectionContext;
+import org.apache.seata.rm.datasource.ConnectionProxy;
+import org.apache.seata.rm.datasource.DataSourceProxy;
+import org.apache.seata.rm.datasource.undo.mysql.MySQLUndoLogManager;
+import org.junit.jupiter.api.Test;
+
+import java.sql.Connection;
+import java.sql.PreparedStatement;
+import java.util.Collections;
+import java.util.concurrent.TimeUnit;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class UndoLogMetricsTest {
+
+ @Test
+ public void testFlushUndoLogsMetrics() throws Exception {
+ Registry registry = mock(Registry.class);
+ org.apache.seata.metrics.Summary summary = mock(org.apache.seata.metrics.Summary.class);
+ when(registry.getSummary(any(Id.class))).thenReturn(summary);
+
+ ConnectionProxy connectionProxy = mock(ConnectionProxy.class);
+ ConnectionContext context = mock(ConnectionContext.class);
+ DataSourceProxy dataSourceProxy = mock(DataSourceProxy.class);
+
+ when(connectionProxy.getContext()).thenReturn(context);
+ when(connectionProxy.getDataSourceProxy()).thenReturn(dataSourceProxy);
+ Connection connection = mock(Connection.class);
+ PreparedStatement preparedStatement = mock(PreparedStatement.class);
+ when(connection.prepareStatement(any(String.class))).thenReturn(preparedStatement);
+ when(connectionProxy.getTargetConnection()).thenReturn(connection);
+ when(dataSourceProxy.getResourceId()).thenReturn("jdbc:mysql://localhost:3306/test");
+
+ when(context.hasUndoLog()).thenReturn(true);
+ when(context.getXid()).thenReturn("xid");
+ when(context.getBranchId()).thenReturn(123456L);
+ when(context.getUndoItems()).thenReturn(Collections.emptyList());
+
+ // Spy on the manager to mock getRegistry()
+ MySQLUndoLogManager manager = spy(new MySQLUndoLogManager());
+ doReturn(registry).when(manager).getRegistry();
+
+ manager.flushUndoLogs(connectionProxy);
+
+ verify(registry).getSummary(eq(UndoLogConstants.SUMMARY_UNDO_LOG_SIZE));
+ verify(summary).increase(anyLong());
+ }
+
+ @Test
+ public void testDeleteUndoLogMetrics() throws Exception {
+ Registry registry = mock(Registry.class);
+ org.apache.seata.metrics.Timer timer = mock(org.apache.seata.metrics.Timer.class);
+ org.apache.seata.metrics.Counter counter = mock(org.apache.seata.metrics.Counter.class);
+
+ when(registry.getTimer(any(Id.class))).thenReturn(timer);
+ when(registry.getCounter(any(Id.class))).thenReturn(counter);
+
+ Connection connection = mock(Connection.class);
+ PreparedStatement preparedStatement = mock(PreparedStatement.class);
+ when(connection.prepareStatement(any(String.class))).thenReturn(preparedStatement);
+
+ // Spy on the manager to mock getRegistry()
+ MySQLUndoLogManager manager = spy(new MySQLUndoLogManager());
+ doReturn(registry).when(manager).getRegistry();
+
+ manager.deleteUndoLog("xid", 123L, connection);
+
+ verify(registry).getTimer(eq(UndoLogConstants.TIMER_UNDO_LOG_DELETE_LATENCY));
+ verify(timer).record(anyLong(), any(TimeUnit.class));
+ verify(registry).getCounter(eq(UndoLogConstants.COUNTER_UNDO_LOG_DELETE_COUNT));
+ verify(counter).increase(1);
+ }
Review Comment:
The production code instruments both `deleteUndoLog` and `batchDeleteUndoLog`, but this test class only verifies metrics for `flushUndoLogs` and the single-row delete path. Add a test that exercises `batchDeleteUndoLog` and asserts the expected counter semantics (especially if the counter is meant to represent operations vs deleted rows).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org