dsmiley commented on code in PR #3734:
URL: https://github.com/apache/solr/pull/3734#discussion_r2421626262
##########
solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java:
##########
@@ -68,117 +75,86 @@ public AttributedInstrumentFactory(
}
public AttributedLongCounter attributedLongCounter(
- MetricNameProvider metricNameProvider, String description, Attributes
additionalAttributes) {
+ String metricName, String description, Attributes additionalAttributes) {
Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes,
additionalAttributes);
if (aggregateToNodeRegistry && nodeMetricsContext != null) {
Attributes finalNodeAttrs = appendAttributes(nodeAttributes,
additionalAttributes);
- LongCounter primaryCounter =
-
primaryMetricsContext.longCounter(metricNameProvider.getPrimaryMetricName(),
description);
+ LongCounter primaryCounter =
primaryMetricsContext.longCounter(metricName, description);
LongCounter nodeCounter =
-
nodeMetricsContext.longCounter(metricNameProvider.getNodeMetricName(),
description);
+ nodeMetricsContext.longCounter(toNodeMetricName(metricName),
description);
return new DualRegistryAttributedLongCounter(
primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs);
} else {
- String metricName =
- primaryIsNodeRegistry
- ? metricNameProvider.getNodeMetricName()
- : metricNameProvider.getPrimaryMetricName();
-
- LongCounter counter = primaryMetricsContext.longCounter(metricName,
description);
+ String finalMetricName = primaryIsNodeRegistry ?
toNodeMetricName(metricName) : metricName;
+ LongCounter counter = primaryMetricsContext.longCounter(finalMetricName,
description);
return new AttributedLongCounter(counter, finalPrimaryAttrs);
}
}
public AttributedLongUpDownCounter attributedLongUpDownCounter(
- MetricNameProvider metricNameProvider, String description, Attributes
additionalAttributes) {
+ String metricName, String description, Attributes additionalAttributes) {
Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes,
additionalAttributes);
if (aggregateToNodeRegistry && nodeMetricsContext != null) {
Attributes finalNodeAttrs = appendAttributes(nodeAttributes,
additionalAttributes);
LongUpDownCounter primaryCounter =
- primaryMetricsContext.longUpDownCounter(
- metricNameProvider.getPrimaryMetricName(), description);
+ primaryMetricsContext.longUpDownCounter(metricName, description);
LongUpDownCounter nodeCounter =
-
nodeMetricsContext.longUpDownCounter(metricNameProvider.getNodeMetricName(),
description);
-
+ nodeMetricsContext.longUpDownCounter(toNodeMetricName(metricName),
description);
return new DualRegistryAttributedLongUpDownCounter(
primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs);
} else {
- String metricName =
- primaryIsNodeRegistry
- ? metricNameProvider.getNodeMetricName()
- : metricNameProvider.getPrimaryMetricName();
-
- LongUpDownCounter counter =
primaryMetricsContext.longUpDownCounter(metricName, description);
+ String finalMetricName = primaryIsNodeRegistry ?
toNodeMetricName(metricName) : metricName;
+ LongUpDownCounter counter =
+ primaryMetricsContext.longUpDownCounter(finalMetricName,
description);
return new AttributedLongUpDownCounter(counter, finalPrimaryAttrs);
}
}
public AttributedLongTimer attributedLongTimer(
- MetricNameProvider metricNameProvider,
- String description,
- OtelUnit unit,
- Attributes additionalAttributes) {
+ String metricName, String description, OtelUnit unit, Attributes
additionalAttributes) {
Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes,
additionalAttributes);
- if (aggregateToNodeRegistry) {
+ if (aggregateToNodeRegistry && nodeMetricsContext != null) {
Attributes finalNodeAttrs = appendAttributes(nodeAttributes,
additionalAttributes);
- LongHistogram coreHistogram =
- primaryMetricsContext.longHistogram(
- metricNameProvider.getPrimaryMetricName(), description, unit);
+ LongHistogram primaryHistogram =
+ primaryMetricsContext.longHistogram(metricName, description, unit);
LongHistogram nodeHistogram =
- nodeMetricsContext.longHistogram(
- metricNameProvider.getNodeMetricName(), description, unit);
+ nodeMetricsContext.longHistogram(toNodeMetricName(metricName),
description, unit);
return new DualRegistryAttributedLongTimer(
- coreHistogram, finalPrimaryAttrs, nodeHistogram, finalNodeAttrs);
+ primaryHistogram, finalPrimaryAttrs, nodeHistogram, finalNodeAttrs);
} else {
- String metricName =
- primaryIsNodeRegistry
- ? metricNameProvider.getNodeMetricName()
- : metricNameProvider.getPrimaryMetricName();
-
- LongHistogram histogram =
primaryMetricsContext.longHistogram(metricName, description, unit);
+ String finalMetricName = primaryIsNodeRegistry ?
toNodeMetricName(metricName) : metricName;
+ LongHistogram histogram =
+ primaryMetricsContext.longHistogram(finalMetricName, description,
unit);
return new AttributedLongTimer(histogram, finalPrimaryAttrs);
}
}
- // Filter out core attributes and keep only category and handler if they
exist
+ // Replace core metric name prefix to node prefix
+ private String toNodeMetricName(String coreMetricName) {
+ return coreMetricName.replace("solr_core", "solr_node");
+ }
+
+ // Filter out core attributes and keep all others for node-level metrics
Review Comment:
Same here; javadocs.
##########
solr/core/src/java/org/apache/solr/metrics/otel/instruments/AttributedInstrumentFactory.java:
##########
@@ -68,117 +75,86 @@ public AttributedInstrumentFactory(
}
public AttributedLongCounter attributedLongCounter(
- MetricNameProvider metricNameProvider, String description, Attributes
additionalAttributes) {
+ String metricName, String description, Attributes additionalAttributes) {
Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes,
additionalAttributes);
if (aggregateToNodeRegistry && nodeMetricsContext != null) {
Attributes finalNodeAttrs = appendAttributes(nodeAttributes,
additionalAttributes);
- LongCounter primaryCounter =
-
primaryMetricsContext.longCounter(metricNameProvider.getPrimaryMetricName(),
description);
+ LongCounter primaryCounter =
primaryMetricsContext.longCounter(metricName, description);
LongCounter nodeCounter =
-
nodeMetricsContext.longCounter(metricNameProvider.getNodeMetricName(),
description);
+ nodeMetricsContext.longCounter(toNodeMetricName(metricName),
description);
return new DualRegistryAttributedLongCounter(
primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs);
} else {
- String metricName =
- primaryIsNodeRegistry
- ? metricNameProvider.getNodeMetricName()
- : metricNameProvider.getPrimaryMetricName();
-
- LongCounter counter = primaryMetricsContext.longCounter(metricName,
description);
+ String finalMetricName = primaryIsNodeRegistry ?
toNodeMetricName(metricName) : metricName;
+ LongCounter counter = primaryMetricsContext.longCounter(finalMetricName,
description);
return new AttributedLongCounter(counter, finalPrimaryAttrs);
}
}
public AttributedLongUpDownCounter attributedLongUpDownCounter(
- MetricNameProvider metricNameProvider, String description, Attributes
additionalAttributes) {
+ String metricName, String description, Attributes additionalAttributes) {
Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes,
additionalAttributes);
if (aggregateToNodeRegistry && nodeMetricsContext != null) {
Attributes finalNodeAttrs = appendAttributes(nodeAttributes,
additionalAttributes);
LongUpDownCounter primaryCounter =
- primaryMetricsContext.longUpDownCounter(
- metricNameProvider.getPrimaryMetricName(), description);
+ primaryMetricsContext.longUpDownCounter(metricName, description);
LongUpDownCounter nodeCounter =
-
nodeMetricsContext.longUpDownCounter(metricNameProvider.getNodeMetricName(),
description);
-
+ nodeMetricsContext.longUpDownCounter(toNodeMetricName(metricName),
description);
return new DualRegistryAttributedLongUpDownCounter(
primaryCounter, finalPrimaryAttrs, nodeCounter, finalNodeAttrs);
} else {
- String metricName =
- primaryIsNodeRegistry
- ? metricNameProvider.getNodeMetricName()
- : metricNameProvider.getPrimaryMetricName();
-
- LongUpDownCounter counter =
primaryMetricsContext.longUpDownCounter(metricName, description);
+ String finalMetricName = primaryIsNodeRegistry ?
toNodeMetricName(metricName) : metricName;
+ LongUpDownCounter counter =
+ primaryMetricsContext.longUpDownCounter(finalMetricName,
description);
return new AttributedLongUpDownCounter(counter, finalPrimaryAttrs);
}
}
public AttributedLongTimer attributedLongTimer(
- MetricNameProvider metricNameProvider,
- String description,
- OtelUnit unit,
- Attributes additionalAttributes) {
+ String metricName, String description, OtelUnit unit, Attributes
additionalAttributes) {
Attributes finalPrimaryAttrs = appendAttributes(primaryAttributes,
additionalAttributes);
- if (aggregateToNodeRegistry) {
+ if (aggregateToNodeRegistry && nodeMetricsContext != null) {
Attributes finalNodeAttrs = appendAttributes(nodeAttributes,
additionalAttributes);
- LongHistogram coreHistogram =
- primaryMetricsContext.longHistogram(
- metricNameProvider.getPrimaryMetricName(), description, unit);
+ LongHistogram primaryHistogram =
+ primaryMetricsContext.longHistogram(metricName, description, unit);
LongHistogram nodeHistogram =
- nodeMetricsContext.longHistogram(
- metricNameProvider.getNodeMetricName(), description, unit);
+ nodeMetricsContext.longHistogram(toNodeMetricName(metricName),
description, unit);
return new DualRegistryAttributedLongTimer(
- coreHistogram, finalPrimaryAttrs, nodeHistogram, finalNodeAttrs);
+ primaryHistogram, finalPrimaryAttrs, nodeHistogram, finalNodeAttrs);
} else {
- String metricName =
- primaryIsNodeRegistry
- ? metricNameProvider.getNodeMetricName()
- : metricNameProvider.getPrimaryMetricName();
-
- LongHistogram histogram =
primaryMetricsContext.longHistogram(metricName, description, unit);
+ String finalMetricName = primaryIsNodeRegistry ?
toNodeMetricName(metricName) : metricName;
+ LongHistogram histogram =
+ primaryMetricsContext.longHistogram(finalMetricName, description,
unit);
return new AttributedLongTimer(histogram, finalPrimaryAttrs);
}
}
- // Filter out core attributes and keep only category and handler if they
exist
+ // Replace core metric name prefix to node prefix
Review Comment:
that is a plain comment but I think should be javadoc. (I merely mean to
tweak the slash style; no need to document anything more)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]