Joscorbe commented on code in PR #2216:
URL: https://github.com/apache/jackrabbit-oak/pull/2216#discussion_r2037069552
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/FullGCStatsCollectorImpl.java:
##########
@@ -37,7 +37,8 @@
*/
class FullGCStatsCollectorImpl implements FullGCStatsCollector {
- static final String FULL_GC = "FullGC";
+ static final String FULL_GC_PUSH_METRICS_PREFIX = "oak_FullGC";
+ static String FULL_GC = "FullGC";
Review Comment:
Do we really need to remove the `final` here?
```suggestion
static final String FULL_GC = "FullGC";
```
##########
oak-run/src/main/java/org/apache/jackrabbit/oak/run/FullGCMetricsExporterFixtureProvider.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.run;
+
+import com.codahale.metrics.MetricRegistry;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.dropwizard.DropwizardExports;
+import io.prometheus.client.exporter.PushGateway;
+import
org.apache.jackrabbit.oak.run.MetricsExporterFixtureProvider.ExportMetricsArgs;
+import
org.apache.jackrabbit.oak.run.MetricsExporterFixtureProvider.ExporterType;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixtureProvider;
+import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+import static java.util.Collections.emptyMap;
+import static
org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils.getService;
+
+/**
+ * Initializes metrics exported fixture for fullGC. For use in oak-run
RevisionsCommand.
+ */
+public class FullGCMetricsExporterFixtureProvider {
+
+ private static final Logger log =
LoggerFactory.getLogger(NodeStoreFixtureProvider.class);
+
+ @Nullable
+ public static FullGCMetricsExporterFixture<PushGateway>
create(RevisionsCommand.RevisionsOptions options, Whiteboard wb) {
Review Comment:
```suggestion
static FullGCMetricsExporterFixture<PushGateway>
create(RevisionsCommand.RevisionsOptions options, Whiteboard wb) {
```
Same here
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/VersionGarbageCollector.java:
##########
@@ -247,9 +248,17 @@ void setFullGcMaxAge(final long fullGcMaxAge, final
TimeUnit unit) {
this.fullGcMaxAgeInMillis = unit.toMillis(fullGcMaxAge);
}
- public void setStatisticsProvider(StatisticsProvider provider) {
+ public void setStatisticsProvider(StatisticsProvider provider) {
Review Comment:
```suggestion
public void setStatisticsProvider(StatisticsProvider provider) {
```
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/FullGCStatsCollectorImpl.java:
##########
@@ -231,12 +238,11 @@ private static CounterStats counter(StatisticsProvider
provider, String name) {
return provider.getCounterStats(qualifiedName(name), METRICS_ONLY);
}
- private static String qualifiedName(String metricName) {
- return FULL_GC + "." + metricName;
+ public static String qualifiedName(String metricName) {
Review Comment:
```suggestion
private static String qualifiedName(String metricName) {
```
I don't see new usages of this method that would require to make it public.
##########
oak-run/src/main/java/org/apache/jackrabbit/oak/run/FullGCMetricsExporterFixtureProvider.java:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.jackrabbit.oak.run;
+
+import com.codahale.metrics.MetricRegistry;
+import io.prometheus.client.CollectorRegistry;
+import io.prometheus.client.dropwizard.DropwizardExports;
+import io.prometheus.client.exporter.PushGateway;
+import
org.apache.jackrabbit.oak.run.MetricsExporterFixtureProvider.ExportMetricsArgs;
+import
org.apache.jackrabbit.oak.run.MetricsExporterFixtureProvider.ExporterType;
+import org.apache.jackrabbit.oak.run.cli.NodeStoreFixtureProvider;
+import org.apache.jackrabbit.oak.spi.whiteboard.Whiteboard;
+import org.jetbrains.annotations.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+
+import static java.util.Collections.emptyMap;
+import static
org.apache.jackrabbit.oak.spi.whiteboard.WhiteboardUtils.getService;
+
+/**
+ * Initializes metrics exported fixture for fullGC. For use in oak-run
RevisionsCommand.
+ */
+public class FullGCMetricsExporterFixtureProvider {
Review Comment:
```suggestion
class FullGCMetricsExporterFixtureProvider {
```
I would suggest to reduce visibility of this class, since it's only intended
in this package.
##########
oak-run/src/main/java/org/apache/jackrabbit/oak/run/RevisionsCommand.java:
##########
@@ -118,7 +125,7 @@ public RevisionsCommand(boolean exitWhenDone) {
this.exitWhenDone = exitWhenDone;
}
- private static class RevisionsOptions extends Utils.NodeStoreOptions {
+ public static class RevisionsOptions extends Utils.NodeStoreOptions {
Review Comment:
```suggestion
static class RevisionsOptions extends Utils.NodeStoreOptions {
```
So with above changes we can reduce this to package-only.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscr...@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
