hanishakoneru commented on a change in pull request #481:
URL: https://github.com/apache/ratis/pull/481#discussion_r650066174
##########
File path:
ratis-metrics/src/main/java/org/apache/ratis/metrics/impl/MetricRegistriesImpl.java
##########
@@ -61,11 +61,6 @@ public MetricRegistriesImpl(MetricRegistryFactory factory) {
@Override
public RatisMetricRegistry create(MetricRegistryInfo info) {
return registries.put(info, () -> {
- if (reporterRegistrations.isEmpty()) {
- LOG.warn(
- "First MetricRegistry has been created without registering
reporters. You may need to call" +
- " MetricRegistries.global().addReporterRegistration(...)
before.");
- }
Review comment:
So without reported being added, I believe there would not be any
metrics to report. But then, it should be fixed from the calling function.
Currently, if we run a load on the system, we keep getting these warning
messages from ratis clients. It does not serve the purpose to spam the client
logs with this warning message, as it needs to fixed on server side.
##########
File path:
ratis-metrics/src/main/java/org/apache/ratis/metrics/RatisMetrics.java
##########
@@ -31,9 +31,16 @@
protected RatisMetricRegistry registry;
protected static RatisMetricRegistry create(MetricRegistryInfo info) {
+ return create(info, true);
+ }
+
+ protected static RatisMetricRegistry create(MetricRegistryInfo info,
+ boolean shouldDebugLog) {
Optional<RatisMetricRegistry> metricRegistry =
MetricRegistries.global().get(info);
return metricRegistry.orElseGet(() -> {
- LOG.info("Creating Metrics Registry : {}", info.getName());
+ if (shouldDebugLog) {
Review comment:
This mertics is used by different components. Currently we were seeing
it create a lot of noise in client logs. Using this `shouldDebugLog` helps with
not printing the message for client logs (even in Debug mode) but printing for
other components such as LeaderElectionMetrics.
For the other components, we can keep it Info or Debug level. I am ok either
way.
##########
File path:
ratis-metrics/src/main/java/org/apache/ratis/metrics/impl/MetricRegistriesImpl.java
##########
@@ -61,11 +61,6 @@ public MetricRegistriesImpl(MetricRegistryFactory factory) {
@Override
public RatisMetricRegistry create(MetricRegistryInfo info) {
return registries.put(info, () -> {
- if (reporterRegistrations.isEmpty()) {
- LOG.warn(
- "First MetricRegistry has been created without registering
reporters. You may need to call" +
- " MetricRegistries.global().addReporterRegistration(...)
before.");
- }
Review comment:
So without reporters being added, I believe there would not be any
metrics to report. But then, it should be fixed from the calling function.
Currently, if we run a load on the system, we keep getting these warning
messages from ratis clients. It does not serve the purpose to spam the client
logs with this warning message, as it needs to fixed on server side.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
