-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
[Docs]OSPP-Add more observability in apollo config client #5229
Conversation
WalkthroughThe pull request updates the Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (3)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (3)
Line range hint
411-411
: Add alternate text to the image.Images should have alternate text for accessibility.
Add meaningful alternate text within the
[]
in the image markdown. For example:![Meaningful alt text describing the image](https://cdn.jsdelivr.net/gh/Rawven/image@main/2024-08-24-14-59-01-image.png)
Tools
LanguageTool
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
Line range hint
542-542
: Add alternate text to the image.Images should have alternate text for accessibility.
Add meaningful alternate text within the
[]
in the image markdown. For example:![Meaningful alt text describing the image](https://raw.githubusercontent.com/Rawven/image/main/20240828003803.png)
Tools
LanguageTool
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
Line range hint
576-576
: Specify the language for the fenced code block.Fenced code blocks should have a language specified for proper syntax highlighting.
Add the language after the opening triple backticks. For example:
```properties # TYPE apollo_client_thread_pool_queue_remaining_capacity gauge ...
Tools
LanguageTool
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- docs/zh/client/java-sdk-user-guide.md (3 hunks)
- docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (1 hunks)
Additional context used
LanguageTool
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
Markdownlint
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
141-141: null
Bare URL used(MD034, no-bare-urls)
docs/zh/client/java-sdk-user-guide.md
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
576-576: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
542-542: null
Images should have alternate text (alt text)(MD045, no-alt-text)
659-659: null
Images should have alternate text (alt text)(MD045, no-alt-text)
Additional comments not posted (1)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (1)
Line range hint
415-415
:Tools
LanguageTool
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
|
||
同时查看Prometheus控制台也能看到如下信息 | ||
|
||
![](https://raw.githubusercontent.com/Rawven/image/main/20240910002345.png) |
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.
Add alternate text to the image.
Images should have alternate text for accessibility.
Add meaningful alternate text within the []
in the image markdown. For example:
![Meaningful alt text describing the image](https://raw.githubusercontent.com/Rawven/image/main/20240910002345.png)
Tools
Markdownlint
659-659: null
Images should have alternate text (alt text)(MD045, no-alt-text)
@Anilople PTAL |
可以先搞定中文文档,最后再翻译到英文文档中 |
15f108b
to
7227498
Compare
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.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (1)
139-142
: Convert the bare URL to a Markdown link.To improve the readability and consistency of the document, consider converting the bare URL to a proper Markdown link.
Apply this diff to convert the bare URL to a Markdown link:
-完整代码:https://github.com/apolloconfig/apollo-java/main/master/apollo-plugin/apollo-plugin-client-prometheus/src/main/java -/com/ctrip/framework/apollo/monitor/internal/exporter/impl/PrometheusApolloClientMetricsExporter.java +完整代码:[PrometheusApolloClientMetricsExporter.java](https://github.com/apolloconfig/apollo-java/main/master/apollo-plugin/apollo-plugin-client-prometheus/src/main/java/com/ctrip/framework/apollo/monitor/internal/exporter/impl/PrometheusApolloClientMetricsExporter.java)Tools
Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
docs/zh/client/java-sdk-user-guide.md (2)
411-412
: Add alternate text to the image.The image showing the JMX monitoring data is relevant. However, please add meaningful alternate text within the
[]
in the image markdown to improve accessibility. For example:![Meaningful alt text describing the image](https://cdn.jsdelivr.net/gh/Rawven/image@main/2024-08-24-14-59-01-image.png)
Tools
Markdownlint
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
415-416
: Improve sentence structure for clarity.The explanation of the
apollo.client.monitor.external.type
configuration property and its usage for enabling exporters is informative. The flexibility of using officially provided or self-implemented MetricsExporter SPI is also mentioned.However, the sentence structure could be slightly improved for better clarity, especially the part "这种设计是为了用户能更方便的扩展". Consider rephrasing it as:
这种设计是为了让用户能更方便地扩展。
The addition of "地" makes the sentence more grammatically correct and easier to understand.
Tools
LanguageTool
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- docs/zh/client/java-sdk-user-guide.md (3 hunks)
- docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (1 hunks)
Additional context used
LanguageTool
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
Markdownlint
docs/zh/client/java-sdk-user-guide.md
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
576-576: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
542-542: null
Images should have alternate text (alt text)(MD045, no-alt-text)
656-656: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
141-141: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (12)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (5)
3-42
: LGTM!The section provides a clear example of creating a custom metrics exporter for Prometheus by extending the
AbstractApolloClientMetricsExporter
class. The code snippet effectively illustrates the structure of the custom exporter.
44-69
: LGTM!The section clearly explains the purpose of the
doInit
method in the custom exporter class and when it is called. The code snippets provide helpful context and an example implementation for initializing the PrometheusCollectorRegistry
and cache map.
71-80
: LGTM!The section clearly explains the purpose of the
isSupport
method in the custom exporter class and when it is called byDefaultApolloClientMetricsExporterFactory
via SPI. The example implementation is straightforward and easy to understand.
82-121
: LGTM!The section clearly explains the purpose of the
registerOrUpdateCounterSample
andregisterOrUpdateGaugeSample
methods in the custom exporter class for registering Counter and Gauge type metrics. The example implementations using the Prometheus client library are straightforward and easy to understand.Tools
LanguageTool
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
123-137
: LGTM!The section clearly explains the purpose of the
response
method in the custom exporter class and how the exported data is used byConfigMonitor
. The example implementation using the PrometheusTextFormat
to write the metrics data is straightforward and easy to understand.docs/zh/client/java-sdk-user-guide.md (7)
402-403
: LGTM!The section introduction for monitoring related configurations looks good.
404-406
: Looks good!The context about the enhanced observability features in version 2.4.0 is clear and informative.
407-408
: Configuration prerequisite is clear.The requirement to set
apollo.client.monitor.enabled
totrue
for enabling the monitor feature is clearly mentioned.
409-410
: JMX configuration property is clear.The
apollo.client.monitor.jmx.enabled
property for exposing monitor data via JMX is clearly mentioned.
413-414
: Exception queue size configuration is clear.The
apollo.client.monitor.exception-queue-size
property for setting the maximum number of exceptions stored by the monitor, along with its default value, is clearly mentioned.
417-418
: Reference to detailed usage section is helpful.Providing a reference to the "Extension Development - Java Client Access to Different Monitoring Systems" section for specific usage details is helpful for users to find more information.
419-420
: Export period configuration is clear.The
apollo.client.monitor.external.export-period
property for controlling the frequency of the scheduled task that exports status information from the monitor, along with its default value, is clearly mentioned.
7227498
to
254c449
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5229 +/- ##
============================================
+ Coverage 51.35% 51.37% +0.01%
- Complexity 2126 2127 +1
============================================
Files 393 393
Lines 12506 12509 +3
Branches 1238 1238
============================================
+ Hits 6422 6426 +4
Misses 5692 5692
+ Partials 392 391 -1 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
docs/zh/client/java-sdk-user-guide.md (1)
415-416
: Consider rephrasing the sentence for better readability.The sentence "这种设计是为了用户能更方便的扩展" could be rephrased as "这种设计是为了用户能更方便地扩展" for better readability.
Tools
LanguageTool
[uncategorized] ~415-~415: 动词的修饰一般为'形容词(副词)+地+动词'。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
Tools
LanguageTool
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- docs/en/client/java-sdk-user-guide.md (4 hunks)
- docs/en/extension/java-client-how-to-use-custom-monitor-system.md (1 hunks)
- docs/zh/client/java-sdk-user-guide.md (3 hunks)
- docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (1 hunks)
Additional context used
LanguageTool
docs/en/client/java-sdk-user-guide.md
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~430-~430: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
[uncategorized] ~82-~82: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...(JI11_JI2)
Markdownlint
docs/en/client/java-sdk-user-guide.md
604-604: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
573-573: null
Images should have alternate text (alt text)(MD045, no-alt-text)
684-684: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/en/extension/java-client-how-to-use-custom-monitor-system.md
141-141: null
Bare URL used(MD034, no-bare-urls)
docs/zh/client/java-sdk-user-guide.md
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
576-576: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
542-542: null
Images should have alternate text (alt text)(MD045, no-alt-text)
656-656: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
141-141: null
Bare URL used(MD034, no-bare-urls)
Additional comments not posted (18)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (5)
1-2
: Helpful overview!The introduction provides a clear and concise overview of the new metrics collection and export feature in the Java client. It sets the context for the changes introduced in this file.
9-40
: LGTM!The
PrometheusApolloClientMetricsExporter
class structure and method signatures are well-defined. The class extends the appropriate base class and implements the required interface. The method names are descriptive and follow the naming conventions.
60-68
: Initialization looks good!The
doInit()
method correctly initializes theCollectorRegistry
and the map to store metrics. The introduction of the Prometheus Java client dependency is justified and necessary for integrating with Prometheus.
84-120
: Implementation looks solid!The
registerOrUpdateCounterSample(...)
andregisterOrUpdateGaugeSample(...)
methods are implemented correctly. The logic for registering and updating counters and gauges follows the best practices of checking for the existence of a metric before creating a new one. The code is readable and well-structured.
125-136
: Response method looks good!The
response()
method is implemented correctly and follows the expected format for Prometheus. The use ofStringWriter
is appropriate for writing the metrics, and the error handling is adequate, logging an error message if the write operation fails.docs/en/extension/java-client-how-to-use-custom-monitor-system.md (1)
1-141
: Excellent documentation for creating a custom metrics exporter!The documentation provides clear step-by-step instructions on how to create a custom metrics exporter for the Apollo Java client to integrate with Prometheus. The code examples are well-structured and cover key aspects such as initialization, metrics registration, and response generation.
This will greatly help users extend the Apollo Java client for monitoring purposes. Great work!
Tools
Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
docs/zh/client/java-sdk-user-guide.md (6)
402-402
: LGTM!The section heading is clear and describes the content that follows.
404-406
: LGTM!The statement provides useful information about the new observability features in version 2.4.0.
407-408
: LGTM!The statement provides clear instructions on how to enable the monitor feature.
409-410
: LGTM!The statement provides clear instructions on how to enable JMX exposure of monitor data.
413-414
: LGTM!The statement provides clear information about the configuration option and its default value.
417-418
: LGTM!The statement provides a reference to where more information about using the feature can be found.
docs/en/client/java-sdk-user-guide.md (6)
417-435
: Excellent addition of monitor-related configurations!The new configuration options significantly enhance the observability of the Apollo client. The defaults are sensible, and the options provide flexibility to customize the monitoring behavior as needed.
Tools
LanguageTool
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~430-~430: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
544-602
: Great usage examples for the monitor feature!The instructions are clear and easy to follow. The configuration examples cover common monitoring scenarios, and the code snippets provide a helpful reference for integrating the feature into a Spring application.
Tools
Markdownlint
573-573: null
Images should have alternate text (alt text)(MD045, no-alt-text)
686-702
: Informative examples of using the ConfigMonitor API!The code segment provides clear examples of how to manually retrieve various types of monitoring data using the ConfigMonitor API. This is helpful for users who need to process and report data to their monitoring systems.
704-729
: Comprehensive metrics data table!The metrics data table provides a good overview of the key metrics exported by the Apollo client. The metrics cover important aspects like configuration usage, thread pool performance, and exceptions. The naming convention for metrics and tags is consistent and descriptive.
1498-1507
: Flexible MetricsExporter extension mechanism!The MetricsExporter extension provides a powerful way for users to integrate Apollo client metrics with their preferred monitoring systems. The SPI and abstract class offer flexibility and simplify the implementation of custom exporters. This extension aligns well with the enhanced observability features in version 2.4.0.
422-422
: Skipping static analysis hints.The flagged punctuation marks are used appropriately in the configuration keys. Specifying a language for the code block and adding alternate text for images would be nice enhancements but are not critical in this context.
Also applies to: 424-424, 428-428, 430-430, 434-434, 604-604, 573-573, 684-684
Tools
LanguageTool
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
docs/en/extension/java-client-how-to-use-custom-monitor-system.md
Outdated
Show resolved
Hide resolved
![](https://cdn.jsdelivr.net/gh/Rawven/image@main/2024-08-24-14-59-01-image.png) | ||
|
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.
Add alternate text to the image.
Images should have alternate text for accessibility.
Add meaningful alternate text within the []
in the image markdown. For example:
![Meaningful alt text describing the image](https://cdn.jsdelivr.net/gh/Rawven/image@main/2024-08-24-14-59-01-image.png)
Tools
Markdownlint
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
Tools
Markdownlint
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
254c449
to
a72df22
Compare
a72df22
to
e2e8c7f
Compare
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (21)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (4)
44-69
: LGTM: Clear explanation and proper initialization indoInit()
.The explanation of the
doInit()
method is clear, and the implementation correctly initializes the PrometheusCollectorRegistry
and a map for storing metrics.Consider adding a brief comment above the
doInit()
method to explain its purpose, like this:/** * Initializes Prometheus-specific objects for metric collection. */ @Override public void doInit() { registry = new CollectorRegistry(); map = new HashMap<>(); }
71-80
: LGTM: Clear explanation and correct implementation ofisSupport()
.The explanation of the
isSupport()
method is clear, and the implementation correctly checks if the exporter supports the "prometheus" form.For improved clarity and maintainability, consider defining a constant for the "prometheus" string:
private static final String PROMETHEUS_FORM = "prometheus"; @Override public boolean isSupport(String form) { return PROMETHEUS_FORM.equals(form); }This makes it easier to update the supported form in the future if needed.
82-121
: LGTM: Correct implementation of Counter and Gauge metric handling.The
registerOrUpdateCounterSample()
andregisterOrUpdateGaugeSample()
methods are implemented correctly, handling both new and existing metrics appropriately. The code follows Prometheus Java client conventions.For consistency, consider using the same pattern for both Counter and Gauge creation methods:
private Counter createCounter(String name, Map<String, String> tags) { return Counter.build() .name(name) .help("Apollo metric: " + name) // Add more descriptive help text .labelNames(tags.keySet().toArray(new String[0])) .register(registry); } private Gauge createGauge(String name, Map<String, String> tags) { return Gauge.build() .name(name) .help("Apollo metric: " + name) // Add more descriptive help text .labelNames(tags.keySet().toArray(new String[0])) .register(registry); }This change adds more descriptive help text for each metric, which can be useful for users of the monitoring system.
123-141
: LGTM: Correct implementation ofresponse()
method and helpful conclusion.The
response()
method is implemented correctly, properly handling the export of metrics in Prometheus format and managing potential IOExceptions. The conclusion and link to the full code are helpful for users who want to see the complete implementation.To address the Markdownlint warning and improve the link's appearance, consider formatting the URL as a proper Markdown link:
完整代码:[PrometheusApolloClientMetricsExporter.java](https://github.com/apolloconfig/apollo-java/main/master/apollo-plugin/apollo-plugin-client-prometheus/src/main/java/com/ctrip/framework/apollo/monitor/internal/exporter/impl/PrometheusApolloClientMetricsExporter.java)This change will make the link more readable and comply with Markdown best practices.
🧰 Tools
🪛 Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
docs/en/extension/java-client-how-to-use-custom-monitor-system.md (4)
9-42
: Consider adding comments for empty method bodies.The class structure is well-defined. However, to improve readability and avoid confusion, consider adding comments in the empty method bodies to indicate that implementations will be provided later in the document.
For example:
@Override public void doInit() { // Implementation provided later in the document }
58-69
: Consider explaining the purpose of the cache map.The Prometheus-specific initialization is well-explained, and the code snippet is clear. However, it would be helpful to briefly explain the purpose of the cache map (map). This would provide readers with a better understanding of its role in the metrics collection process.
For example, you could add:
// map is used to cache metric collectors for efficient lookup and updates private Map<String, Collector.Describable> map;
71-80
: Add a note about the PROMETHEUS constant.The explanation of the isSupport method is clear, and the code snippet effectively demonstrates its implementation. However, the PROMETHEUS constant used in the isSupport method is not defined in the visible context.
Consider adding a note to explain where this constant is defined, or include its definition in the code snippet. For example:
private static final String PROMETHEUS = "prometheus"; @Override public boolean isSupport(String form) { return PROMETHEUS.equals(form); }This will provide readers with a complete understanding of the method's implementation.
123-141
: LGTM: Clear implementation of the response method. Fix the link format.The explanation and implementation of the response method are clear and effective. The method correctly exports data in the Prometheus format and handles potential IOExceptions.
However, the link to the full code at the end of the document is not properly formatted as a Markdown link. To fix this, replace:
Full code:[code](https://github.com/apolloconfig/apollo-java/main/master/apollo-plugin/apollo-plugin-client-prometheus/src/main/java/com/ctrip/framework/apollo/monitor/internal/exporter/impl/PrometheusApolloClientMetricsExporter.java)with:
[Full code](https://github.com/apolloconfig/apollo-java/main/master/apollo-plugin/apollo-plugin-client-prometheus/src/main/java/com/ctrip/framework/apollo/monitor/internal/exporter/impl/PrometheusApolloClientMetricsExporter.java)This will create a proper Markdown link and improve the document's formatting.
docs/zh/client/java-sdk-user-guide.md (7)
413-420
: Consider clarifying the external monitoring configuration optionThe
apollo.client.monitor.external.type
configuration is a good addition for supporting different monitoring systems. However, the name might not be immediately clear to all users. Consider renaming it to something more descriptive, such asapollo.client.monitor.exporter.type
orapollo.client.monitor.metrics.exporter
.Also, it would be helpful to provide a list of built-in exporter types (e.g., "prometheus") in the documentation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
514-581
: Excellent addition of Monitor functionalityThe introduction of the Monitor functionality in version 2.4.0 is a significant enhancement to the Apollo client's observability. The examples provided for using the Monitor API, configuring JMX exposure, and integrating with Prometheus are clear and helpful.
One suggestion for improvement:
Consider adding a brief explanation of the benefits of using this monitoring feature, such as easier troubleshooting, performance optimization, or integration with existing monitoring systems. This would help users understand the value of implementing these new features.🧰 Tools
🪛 Markdownlint
542-542: null
Images should have alternate text (alt text)(MD045, no-alt-text)
582-658
: Comprehensive Prometheus metrics exampleThe provided Prometheus metrics example is excellent, offering a wide range of metrics that cover various aspects of the Apollo client's performance and usage. This will be very useful for users integrating Apollo with their Prometheus monitoring setup.
To improve the documentation:
- Add a language specifier to the code block for proper syntax highlighting. For Prometheus metrics, you can use "promql" or "prometheus":
# TYPE apollo_client_thread_pool_queue_remaining_capacity gauge # HELP apollo_client_thread_pool_queue_remaining_capacity apollo gauge metrics apollo_client_thread_pool_queue_remaining_capacity{thread_pool_name="RemoteConfigRepository"} 2.147483647E9 ...
- Consider adding a brief explanation of some key metrics and what they represent, to help users understand which metrics are most important to monitor.
🧰 Tools
🪛 Markdownlint
582-582: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
659-713
: Well-organized metrics tables and introduction of SPI extensionsThe metrics data tables provide a clear and concise overview of the available metrics, which is very helpful for users. The introduction of SPI extensions for ConfigService load balancing and MetricsExporter adds valuable customization options for advanced users.
Suggestions for improvement:
- For the ConfigService load balancing SPI, consider adding a brief example of how to implement a custom load balancing strategy.
- For the MetricsExporter SPI, provide a short code snippet demonstrating how to create a custom exporter.
- Add links to more detailed documentation or examples for implementing these SPI extensions, if available.
These additions would make it easier for users to understand and implement custom extensions.
🧰 Tools
🪛 Markdownlint
662-662: null
Images should have alternate text (alt text)(MD045, no-alt-text)
411-411
: Add alt text to the imageTo improve accessibility and comply with Markdown best practices, please add alternative text to the image. You can do this by modifying the Markdown as follows:
![JProfiler showing Apollo client metrics](https://raw.githubusercontent.com/Rawven/image/main/20240828003803.png)Replace "JProfiler showing Apollo client metrics" with a brief, descriptive text that explains what the image shows.
🧰 Tools
🪛 Markdownlint
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
415-415
: Correct the Chinese grammarIn the Chinese text, there's a minor grammatical issue. The correct form for modifying a verb with an adverb is "adverb + 地 + verb". Please change:
这种设计是为了用户能更方便的扩展。
to:
这种设计是为了用户能更方便地扩展。
This small change will improve the grammatical correctness of the sentence.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
582-582
: Specify language for the code blockTo improve the formatting and enable syntax highlighting, please specify a language for the fenced code block. For Prometheus metrics, you can use "promql" or "prometheus". Modify the opening of the code block as follows:
```promqlThis will enhance the readability of the metrics example.
🧰 Tools
🪛 Markdownlint
582-582: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
docs/en/client/java-sdk-user-guide.md (6)
417-436
: Excellent addition of monitoring configuration options!This new section on Monitor-related Configuration is a valuable addition for users of Apollo client version 2.4.0 and above. It introduces several useful configuration options to enhance observability. Here are some suggestions for minor improvements:
- Consider using consistent formatting for configuration keys. You could enclose them in backticks (`) for better readability.
- The explanation for
apollo.client.monitor.external.type
mentions "official or custom implementations". It might be helpful to provide a link or reference to where users can find more information about these implementations.Overall, this is a great addition to the documentation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~430-~430: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
Line range hint
438-446
: Consider updating the Apollo client versionThe Maven dependency section is helpful for users integrating Apollo client into their projects. However, the specified version (1.7.0) seems outdated, especially considering the new features mentioned for version 2.4.0 earlier in the document.
Consider updating the version number to the latest stable release that includes the new monitoring features. For example:
<dependency> <groupId>com.ctrip.framework.apollo</groupId> <artifactId>apollo-client</artifactId> <version>2.4.0</version> </dependency>This will ensure users have access to the latest features and improvements.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~430-~430: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
544-744
: Excellent documentation of new monitoring featuresThis new section on Using the Monitor Feature is a great addition to the documentation. It clearly explains how to enable and use the new monitoring capabilities introduced in version 2.4.0, including JMX and Prometheus integration. The code snippets, configuration examples, and metrics data table are particularly helpful for users implementing these features.
One minor suggestion for improvement:
Consider using language-specific code block formatting for better syntax highlighting. For example, replace:
apollo: client: monitor: enabled: true
with:
apollo: client: monitor: enabled: trueThis will enable YAML syntax highlighting for better readability.
🧰 Tools
🪛 Markdownlint
614-614: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
573-573: null
Images should have alternate text (alt text)(MD045, no-alt-text)
694-694: null
Images should have alternate text (alt text)(MD045, no-alt-text)
1506-1518
: Valuable information on apollo-client customizationThis section on apollo-client customization provides useful information for advanced users who need to tailor the client's behavior to their specific requirements. The explanations of the ConfigService load balancing algorithm and MetricsExporter extension are clear and informative.
To further enhance this section, consider adding brief code examples for each customization option. For instance:
- For the ConfigService load balancing algorithm:
public class CustomConfigServiceLoadBalancer implements ConfigServiceLoadBalancerClient { @Override public ServiceDTO getConfigService(List<ServiceDTO> configServices) { // Custom implementation } }
- For the MetricsExporter extension:
public class CustomMetricsExporter extends AbstractApolloClientMetricsExporter { @Override protected void doExport() { // Custom implementation } }These examples would give users a starting point for implementing their own customizations.
Line range hint
1520-1612
: Address relevant static analysis suggestionsThe static analysis tools have flagged some issues, most of which are minor or potentially false positives. However, there are a couple of suggestions worth considering:
Code block language specification: For better syntax highlighting and readability, consider specifying the language for fenced code blocks. For example, use
yaml for YAML configurations and
java for Java code snippets.Image alt text: Add alternative text to images for improved accessibility. For example:
![JProfiler monitoring view](https://raw.githubusercontent.com/Rawven/image/main/20240828003803.png)Implementing these suggestions will improve the overall quality and accessibility of the documentation.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~430-~430: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
Line range hint
1-1612
: Excellent comprehensive documentation with valuable updatesThis document provides a thorough and well-structured guide to using the Apollo client, covering everything from basic setup to advanced customization options. The recent additions, particularly the sections on monitoring features introduced in version 2.4.0, significantly enhance the value of this documentation for users.
Overall, the content is of high quality and will be very helpful for developers working with Apollo. To further improve the document, consider the following minor suggestions:
- Ensure consistency in version numbers throughout the document. Some sections mention version 2.4.0, while others (like the Maven dependency) use older versions.
- Standardize the formatting of configuration keys and code snippets, using consistent syntax highlighting and backticks where appropriate.
- Review and update any links to external resources to ensure they are current and functional.
These small improvements will help maintain the high quality of this already excellent documentation.
🧰 Tools
🪛 Markdownlint
614-614: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
573-573: null
Images should have alternate text (alt text)(MD045, no-alt-text)
694-694: null
Images should have alternate text (alt text)(MD045, no-alt-text)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- docs/en/client/java-sdk-user-guide.md (4 hunks)
- docs/en/extension/java-client-how-to-use-custom-monitor-system.md (1 hunks)
- docs/zh/client/java-sdk-user-guide.md (3 hunks)
- docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/client/java-sdk-user-guide.md
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~430-~430: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~434-~434: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~415-~415: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
🪛 Markdownlint
docs/en/client/java-sdk-user-guide.md
614-614: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
573-573: null
Images should have alternate text (alt text)(MD045, no-alt-text)
694-694: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/zh/client/java-sdk-user-guide.md
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
582-582: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
542-542: null
Images should have alternate text (alt text)(MD045, no-alt-text)
662-662: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
141-141: null
Bare URL used(MD034, no-bare-urls)
🔇 Additional comments (7)
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md (2)
1-13
: LGTM: Clear introduction and proper class definition.The introduction clearly states the purpose of the new feature, and the class definition for
PrometheusApolloClientMetricsExporter
is correctly structured, extendingAbstractApolloClientMetricsExporter
and implementingApolloClientMetricsExporter
.
1-141
: Excellent documentation for custom monitoring implementation.This document provides a comprehensive and well-structured guide for implementing a custom monitoring system, specifically a Prometheus-based metrics exporter, for the Apollo Java client. The explanations are clear, and the code snippets are correct and illustrative.
Key strengths of this documentation:
- Clear introduction and purpose statement
- Step-by-step explanation of each method's implementation
- Proper code snippets with explanations
- Helpful conclusion with a link to the full code
The minor suggestions provided in the review comments will further enhance the clarity, consistency, and adherence to best practices. Overall, this documentation will be very useful for users looking to implement custom monitoring for their Apollo Java client.
🧰 Tools
🪛 Markdownlint
141-141: null
Bare URL used(MD034, no-bare-urls)
docs/en/extension/java-client-how-to-use-custom-monitor-system.md (3)
1-8
: LGTM: Clear introduction and example setup.The introduction effectively sets the context for the new feature in version 2.4.0 and above. The Prometheus integration example is a good choice for demonstrating the implementation.
44-56
: LGTM: Clear explanation of doInit method.The explanation of the doInit method and its usage in AbstractApolloClientMetricsExporter is clear and concise. The provided code snippet effectively demonstrates how doInit is called within the init method.
82-121
: LGTM: Comprehensive implementation of metric registration methods.The explanations and implementations of registerOrUpdateCounterSample and registerOrUpdateGaugeSample methods are clear and comprehensive. The code snippets effectively demonstrate how to register and update Counter and Gauge metrics, including the creation of new metrics when they don't exist in the cache.
docs/zh/client/java-sdk-user-guide.md (1)
401-412
: New monitoring features introducedThese new configuration options for Apollo client monitoring in version 2.4.0+ significantly enhance the client's observability. They allow users to enable monitoring, expose data via JMX, and control exception queue size. This is a valuable addition for users who need better insights into their Apollo client's behavior.
🧰 Tools
🪛 Markdownlint
411-411: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/en/client/java-sdk-user-guide.md (1)
Line range hint
448-473
: Comprehensive overview of client usage approachesThis section provides an excellent overview of the different ways to use Apollo client. The comparison between the API approach and Spring integration approach is particularly helpful for users to choose the most suitable method for their needs. The mention of Spring Boot integration and the reference to additional usage scenarios and sample code are valuable additions.
The information in this section is well-organized and comprehensive, offering users a clear understanding of their options when using Apollo client.
36ea6d4
to
6827b6f
Compare
6827b6f
to
1b72421
Compare
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.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
docs/zh/client/java-sdk-user-guide.md (4)
401-417
: Consider adding a brief introduction to the monitoring featureThe new section about monitoring configurations is informative. However, it would be helpful to add a brief introduction explaining the purpose and benefits of the monitoring feature before diving into the specific configurations. This will provide context for users and help them understand why they might want to use these settings.
Additionally, consider using consistent formatting for configuration keys. For example, you could use backticks (`) to highlight the configuration keys throughout the section.
Here's a suggested improvement for the beginning of this section:
#### 1.2.4.9 Monitor相关配置 > 适用于2.4.0及以上版本 Apollo客户端在2.4.0版本中引入了增强的监控功能,允许用户更好地观察和管理客户端的状态。以下是相关的配置选项: - `apollo.client.monitor.enabled`:是否启动Monitor机制,即ConfigMonitor是否启用,默认为false - `apollo.client.monitor.jmx.enabled`:是否将Monitor数据以JMX形式暴露,开启后可以通过JConsole、JProfiler等工具查看相关信息,默认为false ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~413-~413: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
512-680
: Improve formatting and clarity in the metrics exporting sectionThe metrics exporting section provides valuable information, but there are some areas where formatting and clarity can be improved:
- Use consistent formatting for code blocks. Specify the language for each code block to enable syntax highlighting. For example:
```yaml apollo: client: monitor: enabled: true2. Add meaningful alt text to images for better accessibility. For example: ```markdown ![JProfiler showing Apollo metrics](https://raw.githubusercontent.com/Rawven/image/main/20241020224657.png)
Consider breaking down the long Prometheus metrics example into smaller, more digestible sections with explanations for each metric type.
Improve the language in some parts for better clarity. For example, change "这样就可以通过ConfigMonitor拿到ExporterData(格式取决于你配置的监控系统),然后暴露端点给Prometheus即可" to "通过这种方式,您可以使用ConfigMonitor获取ExporterData(其格式取决于您配置的监控系统),然后将端点暴露给Prometheus进行监控。"
These improvements will enhance the readability and usability of the documentation.
🧰 Tools
🪛 Markdownlint
580-580: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
540-540: null
Images should have alternate text (alt text)(MD045, no-alt-text)
660-660: null
Images should have alternate text (alt text)(MD045, no-alt-text)
413-413
: Correct grammar for better readabilityOn line 413, there's a minor grammatical issue. The correct form should use "地" instead of "的" when modifying a verb. Please change:
- 这种设计是为了用户能更方便的扩展。 + 这种设计是为了用户能更方便地扩展。This small change improves the grammatical correctness of the sentence.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~413-~413: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
511-512
: Add a transition between monitoring configuration and metrics exporting sectionsTo improve the flow of the document, consider adding a brief transition between the monitoring configuration section and the metrics exporting section. This will help readers understand the relationship between these two features. For example:
... 现在我们已经了解了如何配置监控功能,接下来让我们看看如何使用这些监控功能并导出指标数据。 ### 3.1.5 使用Monitor功能 Apollo客户端在2.4.0版本里大幅增强了可观测性,提供了ConfigMonitor-API以及JMX、Prometheus的指标导出方式。
This transition helps to connect the configuration section with the usage section, providing a smoother reading experience.
docs/en/client/java-sdk-user-guide.md (3)
417-434
: Excellent addition of monitoring configurations!The new monitor-related configurations for Apollo Client version 2.4.0+ are well-documented and provide valuable options for enhancing observability. Each configuration is clearly explained with its purpose and default value.
Consider adding a brief example of how to enable these configurations in a typical setup to make it even more user-friendly.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~426-~426: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~432-~432: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
542-611
: Clear instructions for using the Monitor featureThe instructions for enabling and using the Monitor feature are well-explained and easy to follow. The inclusion of code examples is particularly helpful.
The YAML code block at lines 549-554 is missing a language specifier. Consider adding
yaml
after the opening fence for better syntax highlighting:-``` +```yaml apollo: client: monitor: enabled: true🧰 Tools
🪛 Markdownlint
571-571: null
Images should have alternate text (alt text)(MD045, no-alt-text)
612-691
: Comprehensive guide for Prometheus integrationThe instructions for exporting metrics via Prometheus are thorough and well-structured. The inclusion of dependency information and configuration steps is very helpful.
The image at line 571 is missing alternative text. Consider adding a descriptive alt text to improve accessibility, for example:
![J-profiler screenshot showing Apollo metrics](https://raw.githubusercontent.com/Rawven/image/main/20241020224657.png)🧰 Tools
🪛 Markdownlint
612-612: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- docs/en/client/java-sdk-user-guide.md (4 hunks)
- docs/zh/client/java-sdk-user-guide.md (3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/en/client/java-sdk-user-guide.md
[uncategorized] ~422-~422: Loose punctuation mark.
Context: ...ations.apollo.client.monitor.enabled
: Enables the Monitor mechanism, i.e., wh...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~424-~424: Loose punctuation mark.
Context: ...se.apollo.client.monitor.jmx.enabled
: Exposes Monitor data in JMX format. If ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~426-~426: Loose punctuation mark.
Context: ...llo.client.monitor.exception-queue-size`: Sets the maximum number of exceptions t...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~428-~428: Loose punctuation mark.
Context: ....apollo.client.monitor.external.type
: Non-standard configuration item, us...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~432-~432: Loose punctuation mark.
Context: ...o.client.monitor.external.export-period`: The Exporter exports status information...(UNLIKELY_OPENING_PUNCTUATION)
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~413-~413: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
🪛 Markdownlint
docs/en/client/java-sdk-user-guide.md
612-612: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
571-571: null
Images should have alternate text (alt text)(MD045, no-alt-text)
692-692: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/zh/client/java-sdk-user-guide.md
580-580: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
540-540: null
Images should have alternate text (alt text)(MD045, no-alt-text)
660-660: null
Images should have alternate text (alt text)(MD045, no-alt-text)
🔇 Additional comments (4)
docs/zh/client/java-sdk-user-guide.md (1)
Line range hint
401-680
: Overall improvement in documentation with new monitoring featuresThe additions to the Apollo Java SDK user guide, particularly the sections on monitoring configuration and metrics exporting, significantly enhance the documentation. These new features provide users with powerful tools for observing and managing their Apollo client's behavior.
The content is generally well-structured and informative. With the suggested improvements in formatting, language, and transitions, the documentation will be even more user-friendly and accessible.
Key points to address:
- Add brief introductions to new sections for context.
- Improve formatting consistency, especially in code blocks.
- Add alt text to images for accessibility.
- Make minor language and grammar improvements.
- Enhance transitions between sections.
These changes will result in a more polished and professional documentation that better serves Apollo users.
🧰 Tools
🪛 Markdownlint
580-580: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
540-540: null
Images should have alternate text (alt text)(MD045, no-alt-text)
660-660: null
Images should have alternate text (alt text)(MD045, no-alt-text)
docs/en/client/java-sdk-user-guide.md (3)
694-710
: Well-documented ConfigMonitor API usage examplesThe examples provided for manually calling the ConfigMonitor API are clear and cover various aspects of monitoring data retrieval. The comments preceding each example help in understanding the purpose of each API call.
712-742
: Comprehensive metrics data tablesThe provided metrics data tables for Namespace and Thread Pool metrics are well-organized and offer a clear overview of available metrics. This information will be valuable for users setting up monitoring systems.
738-742
: Clear exception metrics tableThe exception metrics table is concise and provides clear information about the available exception-related metric. This complements the previous metrics tables well.
docs/zh/extension/java-client-how-to-use-custom-monitor-system.md
Outdated
Show resolved
Hide resolved
Co-authored-by: wxq <[email protected]>
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.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
docs/zh/client/java-sdk-user-guide.md (1)
401-418
: Fix grammatical issue and improve formatting
- Line 413: "方便的扩展" should be "方便地扩展" according to Chinese grammar rules
- Consider adding a table to better organize the configuration properties and their descriptions
-这种设计是为了用户能更方便的扩展 +这种设计是为了用户能更方便地扩展🧰 Tools
🪛 LanguageTool
[uncategorized] ~413-~413: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- docs/zh/client/java-sdk-user-guide.md (3 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~413-~413: 动词的修饰一般为‘形容词(副词)+地+动词’。您的意思是否是:方便"地"扩展
Context: ...Exporter的SPI使可用官方提供的或自己实现),这种设计是为了用户能更方便的扩展。多填,错填和不填则不启用任何Exporter。 具体使用见 扩展开发-j...(wb4)
🪛 Markdownlint
docs/zh/client/java-sdk-user-guide.md
580-580: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
540-540: null
Images should have alternate text (alt text)(MD045, no-alt-text)
660-660: null
Images should have alternate text (alt text)(MD045, no-alt-text)
9bcc0f2
to
da51124
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/zh/client/java-sdk-user-guide.md (2)
403-427
: Improve configuration documentation format and clarityThe monitoring configuration properties should be consolidated into a single properties block with clear descriptions:
# 1. Enable Monitor mechanism, default is false apollo.client.monitor.enabled=true # 2. Enable JMX metrics exposure, default is false apollo.client.monitor.jmx.enabled=true # 3. Maximum number of exceptions to store, default is 25, follows FIFO principle apollo.client.monitor.exception-queue-size=30 # 4. Specify metrics exporter type, e.g. 'prometheus' when apollo-plugin-client-prometheus is included apollo.client.monitor.external.type=prometheus # 5. Export period in seconds for metrics collection, default is 10 seconds apollo.client.monitor.external.export-period=20
621-635
: Add error handling to the monitoring API exampleThe code example should include error handling and follow consistent formatting:
ConfigMonitor configMonitor = ConfigService.getConfigMonitor(); if (configMonitor == null) { // Handle case when monitoring is not enabled return; } try { // Error monitoring API ApolloClientExceptionMonitorApi exceptionMonitorApi = configMonitor.getExceptionMonitorApi(); List<Exception> apolloConfigExceptionList = exceptionMonitorApi.getApolloConfigExceptionList(); // Namespace monitoring API ApolloClientNamespaceMonitorApi namespaceMonitorApi = configMonitor.getNamespaceMonitorApi(); List<String> namespace404 = namespaceMonitorApi.getNotFoundNamespaces(); // Bootstrap args monitoring API ApolloClientBootstrapArgsMonitorApi runningParamsMonitorApi = configMonitor.getBootstrapArgsMonitorApi(); String bootstrapNamespaces = runningParamsMonitorApi.getBootstrapNamespaces(); // Thread pool monitoring API ApolloClientThreadPoolMonitorApi threadPoolMonitorApi = configMonitor.getThreadPoolMonitorApi(); ApolloThreadPoolInfo remoteConfigRepositoryThreadPoolInfo = threadPoolMonitorApi.getRemoteConfigRepositoryThreadPoolInfo(); } catch (Exception e) { // Handle exceptions appropriately logger.error("Failed to get monitoring metrics", e); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
doc/images/apollo-client-monitor-exporter-load.jpg
is excluded by!**/*.jpg
📒 Files selected for processing (2)
docs/en/client/java-sdk-user-guide.md
(1 hunks)docs/zh/client/java-sdk-user-guide.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
docs/zh/client/java-sdk-user-guide.md
654-654: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
656-656: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
668-668: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
684-684: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
654-654: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
654-654: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
616-616: null
Link fragments should be valid
(MD051, link-fragments)
652-652: null
Link fragments should be valid
(MD051, link-fragments)
1465-1465: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~1700-~1700: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...
(JI11_JI2)
9dba08a
to
f7166bf
Compare
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docs/zh/client/java-sdk-user-guide.md (2)
411-427
: Improve configuration properties formatting for better readability.Group all properties in a single code block with descriptive comments:
# 1. Enable Monitor mechanism, default is false apollo.client.monitor.enabled=true # 2. Enable JMX metrics exposure, default is false apollo.client.monitor.jmx.enabled=true # 3. Maximum number of exceptions to store, default is 25, follows FIFO principle apollo.client.monitor.exception-queue-size=30 # 4. Specify metrics exporter type, e.g. 'prometheus' when apollo-plugin-client-prometheus is included apollo.client.monitor.external.type=prometheus # 5. Export period in seconds for metrics collection, default is 10 seconds apollo.client.monitor.external.export-period=20
Line range hint
429-500
: Enhance the introduction of ConfigMap cache feature.The ConfigMap cache feature introduction could be clearer. Consider adding a brief overview paragraph explaining:
- The purpose of ConfigMap caching
- When to use this feature
- Benefits compared to file-based caching
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
doc/images/apollo-client-monitor-exporter-load.jpg
is excluded by!**/*.jpg
📒 Files selected for processing (1)
docs/zh/client/java-sdk-user-guide.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
docs/zh/client/java-sdk-user-guide.md
654-654: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
656-656: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
668-668: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
684-684: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
654-654: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
654-654: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
616-616: null
Link fragments should be valid
(MD051, link-fragments)
652-652: null
Link fragments should be valid
(MD051, link-fragments)
1465-1465: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~1702-~1702: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...
(JI11_JI2)
🔇 Additional comments (3)
docs/zh/client/java-sdk-user-guide.md (3)
648-648
: Add alt text to the JMX monitoring image for accessibility.
Add meaningful alt text to the image:
-![](https://cdn.jsdelivr.net/gh/apolloconfig/apollo@master/doc/images/apollo-client-monitor-jmx.jpg)
+![JProfiler showing Apollo client monitoring metrics in JMX](https://cdn.jsdelivr.net/gh/apolloconfig/apollo@master/doc/images/apollo-client-monitor-jmx.jpg)
1433-1437
: Use stable version instead of SNAPSHOT in dependency example.
Replace the SNAPSHOT version with a stable release version:
<dependency>
<groupId>com.ctrip.framework.apollo</groupId>
<artifactId>apollo-plugin-client-prometheus</artifactId>
- <version>2.4.0-SNAPSHOT</version>
+ <version>2.4.0</version>
</dependency>
1578-1692
: Improve code formatting in the SkyWalking example.
The code example has formatting issues:
- Use spaces instead of tabs for indentation
- Follow consistent spacing around operators
- Use consistent brace placement
Example of properly formatted code:
public class SkyWalkingMetricsExporter extends AbstractApolloClientMetricsExporter {
private static final String SKYWALKING = "skywalking";
private SkywalkingMeterRegistry registry;
private Map<String, Counter> counterMap;
private Map<String, Gauge> gaugeMap;
private Map<String, AtomicReference<Double>> gaugeValues;
@Override
public void doInit() {
registry = new SkywalkingMeterRegistry();
counterMap = new ConcurrentHashMap<>();
gaugeValues = new ConcurrentHashMap<>();
gaugeMap = new ConcurrentHashMap<>();
}
// ... rest of the implementation
}
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/zh/client/java-sdk-user-guide.md (3)
411-427
: Improve configuration properties formatting for better readability.Group all related properties together in a single properties block with consistent formatting and descriptive comments:
# Enable Monitor mechanism, default is false apollo.client.monitor.enabled=true # Enable JMX metrics exposure, default is false apollo.client.monitor.jmx.enabled=true # Maximum number of exceptions to store, default is 25, follows FIFO principle apollo.client.monitor.exception-queue-size=30 # Specify metrics exporter type, e.g. 'prometheus' when apollo-plugin-client-prometheus is included apollo.client.monitor.external.type=prometheus # Export period in seconds for metrics collection, default is 10 seconds apollo.client.monitor.external.export-period=20
621-635
: Add error handling and improve code formatting in the monitoring API example.The code example should include error handling and follow consistent formatting:
try { ConfigMonitor configMonitor = ConfigService.getConfigMonitor(); if (configMonitor == null) { // Handle case when monitoring is not enabled return; } // Error monitoring API ApolloClientExceptionMonitorApi exceptionMonitorApi = configMonitor.getExceptionMonitorApi(); List<Exception> apolloConfigExceptionList = exceptionMonitorApi.getApolloConfigExceptionList(); // Namespace monitoring API ApolloClientNamespaceMonitorApi namespaceMonitorApi = configMonitor.getNamespaceMonitorApi(); List<String> namespace404 = namespaceMonitorApi.getNotFoundNamespaces(); // Bootstrap args monitoring API ApolloClientBootstrapArgsMonitorApi runningParamsMonitorApi = configMonitor.getBootstrapArgsMonitorApi(); String bootstrapNamespaces = runningParamsMonitorApi.getBootstrapNamespaces(); // Thread pool monitoring API ApolloClientThreadPoolMonitorApi threadPoolMonitorApi = configMonitor.getThreadPoolMonitorApi(); ApolloThreadPoolInfo remoteConfigRepositoryThreadPoolInfo = threadPoolMonitorApi.getRemoteConfigRepositoryThreadPoolInfo(); } catch (Exception e) { // Handle exceptions appropriately logger.error("Failed to get monitoring metrics", e); }
1451-1461
: Improve the example controller implementation.The controller implementation should include error handling and proper response type:
@RestController public class MetricsController { @GetMapping(value = "/metrics", produces = "text/plain") public ResponseEntity<String> metrics() { try { ConfigMonitor configMonitor = ConfigService.getConfigMonitor(); if (configMonitor == null) { return ResponseEntity.status(HttpStatus.SERVICE_UNAVAILABLE) .body("Monitoring not enabled"); } return ResponseEntity.ok(configMonitor.getExporterData()); } catch (Exception e) { return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) .body("Error getting metrics: " + e.getMessage()); } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/en/client/java-sdk-user-guide.md
(3 hunks)docs/zh/client/java-sdk-user-guide.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
docs/zh/client/java-sdk-user-guide.md
654-654: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
656-656: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
668-668: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
684-684: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
654-654: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
654-654: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
616-616: null
Link fragments should be valid
(MD051, link-fragments)
652-652: null
Link fragments should be valid
(MD051, link-fragments)
1551-1551: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1552-1552: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1553-1553: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1554-1554: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1555-1555: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1465-1465: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/en/client/java-sdk-user-guide.md
680-680: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
682-682: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
694-694: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
710-710: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
680-680: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
680-680: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
643-643: null
Link fragments should be valid
(MD051, link-fragments)
678-678: null
Link fragments should be valid
(MD051, link-fragments)
1607-1607: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1608-1608: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1609-1609: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1610-1610: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1611-1611: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1521-1521: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~1702-~1702: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...
(JI11_JI2)
🔇 Additional comments (1)
docs/en/client/java-sdk-user-guide.md (1)
Line range hint 1-1821
: Synchronize English documentation with Chinese version.
The English documentation needs to be updated to include the following sections that were added to the Chinese version:
- Section 1.2.4.9: Client monitoring configuration
- Section 3.1.5: Getting client monitoring metrics
- Section 7.2: Metrics export to Prometheus
- Section 7.3: Metrics export to custom monitoring systems
Please update the English documentation to maintain consistency between both language versions.
Run the following script to verify the missing sections:
✅ Verification successful
The script output shows that both Chinese and English versions contain section 1.2.4.9 about client monitoring. The English version appears to be synchronized with the Chinese version, containing the same key sections about client monitoring capabilities added in version 2.4.0.
Let me verify the other sections as well:
Based on the script output, I can see that all the mentioned sections (1.2.4.9, 3.1.5, 7.2, and 7.3) are present in both the Chinese and English versions of the documentation. The sections cover client monitoring configuration, retrieving monitoring metrics, and metrics export to Prometheus and custom monitoring systems. The content structure and headings match between both versions.
The documentation is already synchronized and no updates are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for missing sections in English documentation
# Check for monitoring sections in both files
echo "Checking Chinese version:"
grep -A 2 "1.2.4.9" docs/zh/client/java-sdk-user-guide.md
echo "Checking English version:"
grep -A 2 "1.2.4.9" docs/en/client/java-sdk-user-guide.md
Length of output: 779
Script:
#!/bin/bash
# Check for section 3.1.5, 7.2 and 7.3 in both versions
echo "Checking Chinese version:"
echo "Section 3.1.5:"
grep -A 2 "3.1.5" docs/zh/client/java-sdk-user-guide.md
echo -e "\nSection 7.2:"
grep -A 2 "7.2" docs/zh/client/java-sdk-user-guide.md
echo -e "\nSection 7.3:"
grep -A 2 "7.3" docs/zh/client/java-sdk-user-guide.md
echo -e "\nChecking English version:"
echo "Section 3.1.5:"
grep -A 2 "3.1.5" docs/en/client/java-sdk-user-guide.md
echo -e "\nSection 7.2:"
grep -A 2 "7.2" docs/en/client/java-sdk-user-guide.md
echo -e "\nSection 7.3:"
grep -A 2 "7.3" docs/en/client/java-sdk-user-guide.md
Length of output: 2738
🧰 Tools
🪛 Markdownlint (0.37.0)
1607-1607: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1608-1608: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1609-1609: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1610-1610: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1611-1611: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1521-1521: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
857b755
to
8693f53
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/zh/client/java-sdk-user-guide.md (4)
411-427
: Improve properties documentation format for better readability.The configuration properties should be formatted consistently with descriptions. Consider using this format:
# Enable Monitor mechanism, default is false apollo.client.monitor.enabled=true # Enable JMX metrics exposure, default is false apollo.client.monitor.jmx.enabled=true # Maximum number of exceptions to store, default is 25, follows FIFO principle apollo.client.monitor.exception-queue-size=30 # Specify metrics exporter type, e.g. 'prometheus' when apollo-plugin-client-prometheus is included apollo.client.monitor.external.type=prometheus # Export period in seconds for metrics collection, default is 10 seconds apollo.client.monitor.external.export-period=20
671-694
: Improve metric tables formatting for better readability.The metric tables should have consistent column widths and alignment. Consider:
- Aligning column headers
- Using consistent spacing
- Adding descriptions for metric types
🧰 Tools
🪛 Markdownlint (0.37.0)
679-679: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
Line range hint
418-442
: Synchronize English documentation with Chinese version.The English documentation is missing several key sections that were added to the Chinese version:
- Section 1.2.4.9: Client monitoring configuration
- Section 3.1.6: Getting client monitoring metrics
- Section 7.2: Metrics export to Prometheus
- Section 7.3: Metrics export to custom monitoring systems
Consider maintaining both language versions simultaneously when making updates to ensure consistency.
632-646
: Improve error handling in monitoring API example.The code example should include proper error handling and consistent formatting:
try { ConfigMonitor configMonitor = ConfigService.getConfigMonitor(); if (configMonitor == null) { // Handle case when monitoring is not enabled return; } // Error monitoring API ApolloClientExceptionMonitorApi exceptionMonitorApi = configMonitor.getExceptionMonitorApi(); List<Exception> apolloConfigExceptionList = exceptionMonitorApi.getApolloConfigExceptionList(); // Namespace monitoring API ApolloClientNamespaceMonitorApi namespaceMonitorApi = configMonitor.getNamespaceMonitorApi(); List<String> namespace404 = namespaceMonitorApi.getNotFoundNamespaces(); // Bootstrap args monitoring API ApolloClientBootstrapArgsMonitorApi runningParamsMonitorApi = configMonitor.getBootstrapArgsMonitorApi(); String bootstrapNamespaces = runningParamsMonitorApi.getBootstrapNamespaces(); // Thread pool monitoring API ApolloClientThreadPoolMonitorApi threadPoolMonitorApi = configMonitor.getThreadPoolMonitorApi(); ApolloThreadPoolInfo remoteConfigRepositoryThreadPoolInfo = threadPoolMonitorApi.getRemoteConfigRepositoryThreadPoolInfo(); } catch (Exception e) { // Handle exceptions appropriately logger.error("Failed to get monitoring metrics", e); }docs/en/client/java-sdk-user-guide.md (1)
651-728
: Complete the English translation of monitoring sections.The English documentation needs the following sections to be completed:
- Client monitoring configuration details
- Metrics export functionality
- Custom monitoring system integration
- Complete metrics tables with descriptions
Would you like me to help generate the missing English documentation sections to maintain consistency with the Chinese version?
🧰 Tools
🪛 Markdownlint (0.37.0)
691-691: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
693-693: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
705-705: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
721-721: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
691-691: Expected: underscore; Actual: asterisk
Emphasis style(MD049, emphasis-style)
691-691: Expected: underscore; Actual: asterisk
Emphasis style(MD049, emphasis-style)
654-654: null
Link fragments should be valid(MD051, link-fragments)
689-689: null
Link fragments should be valid(MD051, link-fragments)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/en/client/java-sdk-user-guide.md
(3 hunks)docs/zh/client/java-sdk-user-guide.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
docs/zh/client/java-sdk-user-guide.md
665-665: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
667-667: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
679-679: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
695-695: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
665-665: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
665-665: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
627-627: null
Link fragments should be valid
(MD051, link-fragments)
663-663: null
Link fragments should be valid
(MD051, link-fragments)
1572-1572: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1573-1573: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1574-1574: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1575-1575: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1576-1576: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1486-1486: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/en/client/java-sdk-user-guide.md
691-691: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
693-693: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
705-705: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
721-721: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
691-691: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
691-691: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
654-654: null
Link fragments should be valid
(MD051, link-fragments)
689-689: null
Link fragments should be valid
(MD051, link-fragments)
1631-1631: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1632-1632: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1633-1633: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1634-1634: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1635-1635: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1545-1545: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~1723-~1723: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...
(JI11_JI2)
8693f53
to
727e9df
Compare
e40dfd4
to
c95708e
Compare
PTAL @Anilople |
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.
感觉中文的没啥问题了
c95708e
to
4538c5a
Compare
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.
Actionable comments posted: 2
🧹 Nitpick comments (5)
docs/zh/client/java-sdk-user-guide.md (1)
411-427
: Improve configuration properties formatting for better readability.The configuration properties should be formatted consistently with descriptions. Consider using this format:
# 1. Enable Monitor mechanism, default is false apollo.client.monitor.enabled=true # 2. Enable JMX metrics exposure, default is false apollo.client.monitor.jmx.enabled=true # 3. Maximum number of exceptions to store, default is 25, follows FIFO principle apollo.client.monitor.exception-queue-size=30 # 4. Specify metrics exporter type, e.g. 'prometheus' when apollo-plugin-client-prometheus is included apollo.client.monitor.external.type=prometheus # 5. Export period in seconds for metrics collection, default is 10 seconds apollo.client.monitor.external.export-period=20docs/en/client/java-sdk-user-guide.md (4)
651-690
: Consider adding example output for better clarity.The section about retrieving monitoring metrics provides good code examples. However, it would be helpful to add example output showing what metrics are available through each API method.
Consider adding example output like:
// Example output from exceptionMonitorApi.getApolloConfigExceptionList(): [ ApolloConfigException: "Failed to load config...", ... ] // Example output from namespaceMonitorApi.getNotFoundNamespaces(): ["namespace1", "namespace2"]🧰 Tools
🪛 Markdownlint (0.37.0)
654-654: null
Link fragments should be valid(MD051, link-fragments)
697-728
: Enhance metric documentation with descriptions and units.The metrics tables would be more useful with additional columns for:
- Description: What each metric represents
- Unit: The unit of measurement (e.g., milliseconds, count)
For example:
| Metric Name | Tag | Description | Unit | Corresponding Monitor-API | |------------|-----|-------------|------|-------------------------| | apollo_client_namespace_first_load_time_spend_in_ms | namespace | Time taken for first load of namespace | milliseconds | namespaceMetrics.getFirstLoadTimeSpendInMs() |🧰 Tools
🪛 Markdownlint (0.37.0)
705-705: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
721-721: null
Emphasis used instead of a heading(MD036, no-emphasis-as-heading)
1630-1636
: Consider adding implementation tips for custom exporters.The section about implementing custom exporters could benefit from additional guidance on:
- Best practices for implementing each method
- Error handling recommendations
- Performance considerations
🧰 Tools
🪛 Markdownlint (0.37.0)
1631-1631: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
1632-1632: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
1633-1633: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
1634-1634: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
1635-1635: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
1674-1676
: Add warning about example code limitations.The warning about not using the example directly in production is good, but consider expanding it to explain:
- What aspects need to be considered for production use
- Potential issues to watch out for
- Links to best practices or reference implementations
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/en/client/java-sdk-user-guide.md
(3 hunks)docs/zh/client/java-sdk-user-guide.md
(3 hunks)
🧰 Additional context used
🪛 Markdownlint (0.37.0)
docs/en/client/java-sdk-user-guide.md
691-691: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
693-693: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
705-705: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
721-721: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
691-691: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
691-691: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
654-654: null
Link fragments should be valid
(MD051, link-fragments)
1631-1631: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1632-1632: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1633-1633: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1634-1634: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1635-1635: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1545-1545: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/zh/client/java-sdk-user-guide.md
665-665: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
667-667: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
679-679: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
695-695: null
Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
665-665: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
665-665: Expected: underscore; Actual: asterisk
Emphasis style
(MD049, emphasis-style)
627-627: null
Link fragments should be valid
(MD051, link-fragments)
1573-1573: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1574-1574: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1575-1575: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1576-1576: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1577-1577: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1486-1486: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/zh/client/java-sdk-user-guide.md
[uncategorized] ~1747-~1747: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...
(JI11_JI2)
🔇 Additional comments (2)
docs/zh/client/java-sdk-user-guide.md (1)
Line range hint 1-1867
: Synchronize English documentation with Chinese version.
The English documentation needs to be updated to include the following sections that were added to the Chinese version:
- Section 1.2.4.9: Client monitoring configuration
- Section 3.1.6: Getting client monitoring metrics
- Section 7.2: Metrics export to Prometheus
- Section 7.3: Metrics export to custom monitoring systems
Please ensure both language versions are kept in sync to maintain documentation consistency.
Would you like me to help draft the English translations for these sections?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~1747-~1747: 您的意思是“"既"是”吗?
Context: ...ounterSample,registerOrUpdateGaugeSample即是用来注册Counter,Gauge类型指标的方法,只需要根据传来的参数正常注册...
(JI11_JI2)
🪛 Markdownlint (0.37.0)
1573-1573: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1574-1574: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1575-1575: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1576-1576: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1577-1577: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
1486-1486: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
docs/en/client/java-sdk-user-guide.md (1)
418-443
: Documentation structure and content look good!
The new section about enabling client monitoring is well-structured and provides clear configuration examples with explanations for each setting. The version requirement is clearly stated.
问题已经解决完毕 ,再看看 @Anilople |
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.
LGTM
What's the purpose of this PR
related pr apolloconfig/apollo-java#74
XXXXX
Which issue(s) this PR fixes:
Fixes #
Brief changelog
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.Summary by CodeRabbit
New Features
Documentation