sk0x50 commented on code in PR #7350:
URL: https://github.com/apache/ignite-3/pull/7350#discussion_r2690851932
##########
modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/log/LogPushExporter.java:
##########
@@ -71,54 +83,373 @@ public void report() {
return;
}
- var report = new StringBuilder("Metric report:");
+ var report = new StringBuilder("Metrics for local node: ");
+
+ appendNodeInfo(report, metricSets);
Review Comment:
I think that you don't need all that stuff at all. :)
The thing that is really needed is adding the corresponding metric sources
to a configuration of `LogPushExporter` as follows:
```
/**
* List of enabled metric sources. If not empty, metric sources that are
not enumerated will not be printed.
* Wildcard '*' can be used in the end of each item. Some metrics are
logged by default. To disable it, specify the empty list here
* explicitly. To print all metrics, include a single string '*'.
*/
@Value(hasDefault = true)
public String[] enabledMetrics = {
"metastorage",
"placement-driver",
"resource.vacuum",
"jvm",
"os",
"topology.local",
"topology.cluster",
"thread.pools.partitions-executor",
"thread.pools.sql-executor"
};
```
See
[LogPushExporterConfigurationSchema](https://github.com/apache/ignite-3/blob/main/modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/configuration/LogPushExporterConfigurationSchema.java)
##########
modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/log/LogPushExporter.java:
##########
@@ -71,54 +83,373 @@ public void report() {
return;
}
- var report = new StringBuilder("Metric report:");
+ var report = new StringBuilder("Metrics for local node: ");
+
+ appendNodeInfo(report, metricSets);
+
+ boolean initialized = isNodeInitialized(metricSets);
+ report.append(", state=").append(initialized ? "initialized" :
"uninitialized");
+
+ UUID clusterId = clusterIdSupplier().get();
+ if (clusterId != null) {
+ report.append(", clusterId=").append(clusterId);
+ }
+
+ int nodeCount = getClusterNodeCount(metricSets);
+ if (nodeCount > 0) {
+ report.append(", topology=").append(nodeCount).append(" nodes");
+ }
+
+ appendNetworkInfo(report, metricSets);
+
+ appendCpuInfo(report, metricSets);
+
+ appendHeapInfo(report, metricSets);
for (MetricSet metricSet : metricSets) {
boolean hasMetricsWhiteList = hasMetricsWhiteList(metricSet);
if (hasMetricsWhiteList || metricEnabled(metricSet.name())) {
-
report.append('\n').append(metricSet.name()).append(oneLinePerMetricSource ? '
' : ':');
-
- appendMetrics(report, metricSet, hasMetricsWhiteList);
+ if (oneLinePerMetricSource) {
+ report.append(", ").append(metricSet.name()).append('=');
+ appendMetricsOneLine(report, metricSet,
hasMetricsWhiteList);
Review Comment:
`appendMetricsOneLine` adds all metric sources to a single line; however,
`oneLinePerMetricSource` means that each metric source should be added to a new
line.
I would expect something like the following, depending on
`oneLinePerMetricSource` value:
`oneLinePerMetricSource == false`
```
...[LogPushExporter] Metrics for local node: jvm [UpTime=38592, ...],
metastorage [IdempotentCacheSize=6, ...], placement-driver [...],
resource.vacuum [...], thread.pools.partitions-executor [...], ...
```
`oneLinePerMetricSource == true`
```
...[LogPushExporter] Metrics for local node:
^-- metastorage [IdempotentCacheSize=6, SafeTimeLag=168]
^-- os [AvailableProcessors=20, CpuLoad=0.2, LoadAverage=0.7802734375]
^-- placement-driver [AcceptedLeases=0, LeaseNegotiations=0,
ReplicationGroups=0]
^-- resource.vacuum [...]
^-- thread.pools.partitions-executor [...]
^-- thread.pools.sql-executor [...]
^-- topology.cluster [ClusterId=a245a6d0-52d1-42fd-b47f-9190e60fc0f9,
ClusterName=cluster, TotalNodes=1]
^-- topology.local [NetworkAddress=127.0.1.1, NetworkPort=3344,
NodeId=7406a904-cc03-4437-a811-0517180d078d, NodeName=node,
NodeVersion=3.2.0-SNAPSHOT]
```
##########
modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/log/LogPushExporter.java:
##########
@@ -42,13 +46,21 @@
public class LogPushExporter extends PushMetricExporter {
public static final String EXPORTER_NAME = "logPush";
- /** Padding for individual metric output. */
+ /** Padding for individual metric output in multiline mode. */
Review Comment:
Let's change the padding value to `" ^-- "`
##########
modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/log/LogPushExporter.java:
##########
@@ -71,54 +83,373 @@ public void report() {
return;
}
- var report = new StringBuilder("Metric report:");
+ var report = new StringBuilder("Metrics for local node: ");
+
+ appendNodeInfo(report, metricSets);
+
+ boolean initialized = isNodeInitialized(metricSets);
+ report.append(", state=").append(initialized ? "initialized" :
"uninitialized");
+
+ UUID clusterId = clusterIdSupplier().get();
+ if (clusterId != null) {
+ report.append(", clusterId=").append(clusterId);
+ }
+
+ int nodeCount = getClusterNodeCount(metricSets);
+ if (nodeCount > 0) {
+ report.append(", topology=").append(nodeCount).append(" nodes");
+ }
+
+ appendNetworkInfo(report, metricSets);
+
+ appendCpuInfo(report, metricSets);
+
+ appendHeapInfo(report, metricSets);
for (MetricSet metricSet : metricSets) {
boolean hasMetricsWhiteList = hasMetricsWhiteList(metricSet);
if (hasMetricsWhiteList || metricEnabled(metricSet.name())) {
-
report.append('\n').append(metricSet.name()).append(oneLinePerMetricSource ? '
' : ':');
-
- appendMetrics(report, metricSet, hasMetricsWhiteList);
+ if (oneLinePerMetricSource) {
+ report.append(", ").append(metricSet.name()).append('=');
+ appendMetricsOneLine(report, metricSet,
hasMetricsWhiteList);
+ } else {
+ report.append('\n').append(metricSet.name()).append(':');
Review Comment:
Please, don't use `\n` as a line separator. The correct way is to use
`System.lineSeparator()`.
##########
modules/metrics/src/main/java/org/apache/ignite/internal/metrics/exporters/log/LogPushExporter.java:
##########
@@ -71,54 +83,373 @@ public void report() {
return;
}
- var report = new StringBuilder("Metric report:");
+ var report = new StringBuilder("Metrics for local node: ");
+
+ appendNodeInfo(report, metricSets);
+
+ boolean initialized = isNodeInitialized(metricSets);
+ report.append(", state=").append(initialized ? "initialized" :
"uninitialized");
+
+ UUID clusterId = clusterIdSupplier().get();
+ if (clusterId != null) {
+ report.append(", clusterId=").append(clusterId);
+ }
+
+ int nodeCount = getClusterNodeCount(metricSets);
+ if (nodeCount > 0) {
+ report.append(", topology=").append(nodeCount).append(" nodes");
+ }
+
+ appendNetworkInfo(report, metricSets);
+
+ appendCpuInfo(report, metricSets);
+
+ appendHeapInfo(report, metricSets);
for (MetricSet metricSet : metricSets) {
boolean hasMetricsWhiteList = hasMetricsWhiteList(metricSet);
if (hasMetricsWhiteList || metricEnabled(metricSet.name())) {
-
report.append('\n').append(metricSet.name()).append(oneLinePerMetricSource ? '
' : ':');
-
- appendMetrics(report, metricSet, hasMetricsWhiteList);
+ if (oneLinePerMetricSource) {
+ report.append(", ").append(metricSet.name()).append('=');
+ appendMetricsOneLine(report, metricSet,
hasMetricsWhiteList);
+ } else {
+ report.append('\n').append(metricSet.name()).append(':');
+ appendMetricsMultiline(report, metricSet,
hasMetricsWhiteList);
+ }
}
}
+ if (oneLinePerMetricSource) {
+ appendThreadPoolMetrics(report, metricSets);
+ }
+
log.info(report.toString());
}
- private void appendMetrics(StringBuilder sb, MetricSet metricSet, boolean
hasMetricsWhiteList) {
- List<Metric> metrics = StreamSupport.stream(metricSet.spliterator(),
false)
- .sorted(comparing(Metric::name))
- .filter(m -> !hasMetricsWhiteList ||
metricEnabled(fqn(metricSet, m)))
- .collect(toList());
+ /**
+ * Appends node information to the report.
+ *
+ * @param report Report string builder.
+ * @param metricSets Collection of metric sets.
+ */
+ private void appendNodeInfo(StringBuilder report, Collection<MetricSet>
metricSets) {
+ String ephemeralId = getEphemeralNodeId(metricSets);
+ String nodeName = nodeName();
+ String version = getNodeVersion(metricSets);
+
+ long uptimeMs = runtimeMxBean.getUptime();
+
+ report.append("Node [");
+
+ boolean needComma = false;
+
+ if (ephemeralId != null && !ephemeralId.isEmpty()) {
+ report.append("id=").append(ephemeralId);
+ needComma = true;
+ }
- sb.append(metricSetPrefix());
+ if (nodeName != null && !nodeName.isEmpty()) {
+ if (needComma) {
+ report.append(", ");
+ }
+ report.append("name=").append(nodeName);
+ needComma = true;
+ }
- forEachIndexed(metrics, (m, i) ->
appendMetricWithValue(oneLinePerMetricSource, sb, m, i));
+ if (version != null && !version.isEmpty()) {
+ if (needComma) {
+ report.append(", ");
+ }
+ report.append("version=").append(version);
+ needComma = true;
+ }
- sb.append(metricSetPostfix());
+ if (needComma) {
+ report.append(", ");
+ }
+ report.append("uptime=").append(formatUptimeHms(uptimeMs))
+ .append(']');
}
- private static String commaInEnum(int i) {
- return i == 0 ? "" : ", ";
+ /**
+ * Checks if the node is initialized based on the presence of key metrics.
+ *
+ * @param metricSets Collection of metric sets.
+ * @return True if the node is initialized, false otherwise.
+ */
+ private boolean isNodeInitialized(Collection<MetricSet> metricSets) {
Review Comment:
It looks like this metric is a bit useless (at least for now), just because
the `LogExporter` starts working when the cluster is already initialized,
therefore, we will never see the `false` value here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
