From 572a745bfb9d51923bd29a4ddb5a3e4a09e42d5a Mon Sep 17 00:00:00 2001 From: Srikanth Padakanti Date: Mon, 30 Mar 2026 11:03:35 -0500 Subject: [PATCH 1/3] Fix NPE in sanitizeMetricName when unit or aggregationTemporality is null Signed-off-by: Srikanth Padakanti --- .../service/PrometheusTimeSeries.java | 42 ++++++++++--------- .../service/PrometheusTimeSeriesTest.java | 25 +++++++++++ 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java b/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java index 423f651056..d1b9f0d402 100644 --- a/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java +++ b/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java @@ -302,38 +302,40 @@ static String sanitizeMetricName(final Metric metric) { final boolean isGauge = metric.getKind().equals(Metric.KIND.GAUGE.toString()); final boolean isCounter = metric.getKind().equals(Metric.KIND.SUM.toString()) && ((Sum) metric).isMonotonic() && - ((Sum) metric).getAggregationTemporality().equals("AGGREGATION_TEMPORALITY_CUMULATIVE"); + "AGGREGATION_TEMPORALITY_CUMULATIVE".equals(((Sum) metric).getAggregationTemporality()); StringBuilder metricNameBuilder = new StringBuilder(sanitizeName(name, true, false)); String suffix = isCounter ? TOTAL_SUFFIX : ""; - if (unit.startsWith("{")) { + if (unit != null && unit.startsWith("{")) { return metricNameBuilder.append(suffix).toString(); } - if ("1".equals(unit) && isGauge) { - return metricNameBuilder.append(RATIO_SUFFIX).toString(); - } + if (unit != null) { + if ("1".equals(unit) && isGauge) { + return metricNameBuilder.append(RATIO_SUFFIX).toString(); + } - String mappedUnit = otelToPrometheusUnitsMap.get(unit); - if (mappedUnit != null) { - return metricNameBuilder.append(UNDERSCORE).append(mappedUnit).append(suffix).toString(); - } + String mappedUnit = otelToPrometheusUnitsMap.get(unit); + if (mappedUnit != null) { + return metricNameBuilder.append(UNDERSCORE).append(mappedUnit).append(suffix).toString(); + } - if (unit.contains("/")) { - String[] unitSplit = unit.split("/", 2); - if (unitSplit.length == 2) { - String unit1 = otelToPrometheusUnitsMap.get(unitSplit[0]); - String unit2 = otelToPrometheusUnitsMap.get(unitSplit[1]); - if (unit1 != null && unit2 != null) { - return metricNameBuilder.append(UNDERSCORE).append(unit1) - .append(UNDERSCORE).append(unit2).append(suffix).toString(); + if (unit.contains("/")) { + String[] unitSplit = unit.split("/", 2); + if (unitSplit.length == 2) { + String unit1 = otelToPrometheusUnitsMap.get(unitSplit[0]); + String unit2 = otelToPrometheusUnitsMap.get(unitSplit[1]); + if (unit1 != null && unit2 != null) { + return metricNameBuilder.append(UNDERSCORE).append(unit1) + .append(UNDERSCORE).append(unit2).append(suffix).toString(); + } } } - } - if (!"1".equals(unit)) { - metricNameBuilder.append(UNDERSCORE).append(unit); + if (!"1".equals(unit)) { + metricNameBuilder.append(UNDERSCORE).append(unit); + } } return metricNameBuilder.append(suffix).toString(); } diff --git a/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java b/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java index e6ff20855f..261ff9f50f 100644 --- a/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java +++ b/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java @@ -12,6 +12,7 @@ import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -187,6 +188,30 @@ public void testHistogramMetricKey(String unit, String expectedUnitName) throws assertTrue(timeSeries.getMetricKey().equals(expectedKey1) || timeSeries.getMetricKey().equals(expectedKey2)); } + @Test + public void testSumMetricWithNullAggregationTemporality() { + final String name = RandomStringUtils.randomAlphabetic(10); + Sum sum = createSumMetric(name, "s", true, null); + String sanitizedName = PrometheusTimeSeries.sanitizeMetricName(sum); + assertThat(sanitizedName, equalTo(name + "_seconds")); + } + + @Test + public void testGaugeMetricWithNullUnit() { + final String name = RandomStringUtils.randomAlphabetic(10); + Gauge gauge = createGaugeMetric(name, null); + String sanitizedName = PrometheusTimeSeries.sanitizeMetricName(gauge); + assertThat(sanitizedName, equalTo(name)); + } + + @Test + public void testSumMetricWithNullUnitAndNullAggregationTemporality() { + final String name = RandomStringUtils.randomAlphabetic(10); + Sum sum = createSumMetric(name, null, true, null); + String sanitizedName = PrometheusTimeSeries.sanitizeMetricName(sum); + assertThat(sanitizedName, equalTo(name)); + } + private Summary createSummaryMetric(final String name, final String unit) { List quantiles = Arrays.asList( new DefaultQuantile(0.5d, 10d), From 204e984d7d4a5fc327982ab8edde0b1801b87ec0 Mon Sep 17 00:00:00 2001 From: Srikanth Padakanti Date: Tue, 31 Mar 2026 11:18:59 -0500 Subject: [PATCH 2/3] Add test for counter Sum metric with null unit Signed-off-by: Srikanth Padakanti --- .../sink/prometheus/service/PrometheusTimeSeriesTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java b/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java index 261ff9f50f..69fc67b6dc 100644 --- a/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java +++ b/data-prepper-plugins/prometheus-sink/src/test/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeriesTest.java @@ -204,6 +204,14 @@ public void testGaugeMetricWithNullUnit() { assertThat(sanitizedName, equalTo(name)); } + @Test + public void testSumMetricCounterWithNullUnit() { + final String name = RandomStringUtils.randomAlphabetic(10); + Sum sum = createSumMetric(name, null, true, "AGGREGATION_TEMPORALITY_CUMULATIVE"); + String sanitizedName = PrometheusTimeSeries.sanitizeMetricName(sum); + assertThat(sanitizedName, equalTo(name + "_total")); + } + @Test public void testSumMetricWithNullUnitAndNullAggregationTemporality() { final String name = RandomStringUtils.randomAlphabetic(10); From 3786679255bad37bfe781b6e227aa6338cb023ab Mon Sep 17 00:00:00 2001 From: Srikanth Padakanti Date: Tue, 31 Mar 2026 15:08:24 -0500 Subject: [PATCH 3/3] Move null unit check to top of sanitizeMetricName Signed-off-by: Srikanth Padakanti --- .../service/PrometheusTimeSeries.java | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java b/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java index d1b9f0d402..b42fbe43c6 100644 --- a/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java +++ b/data-prepper-plugins/prometheus-sink/src/main/java/org/opensearch/dataprepper/plugins/sink/prometheus/service/PrometheusTimeSeries.java @@ -307,35 +307,37 @@ static String sanitizeMetricName(final Metric metric) { StringBuilder metricNameBuilder = new StringBuilder(sanitizeName(name, true, false)); String suffix = isCounter ? TOTAL_SUFFIX : ""; - if (unit != null && unit.startsWith("{")) { + if (unit == null) { return metricNameBuilder.append(suffix).toString(); } - if (unit != null) { - if ("1".equals(unit) && isGauge) { - return metricNameBuilder.append(RATIO_SUFFIX).toString(); - } + if (unit.startsWith("{")) { + return metricNameBuilder.append(suffix).toString(); + } - String mappedUnit = otelToPrometheusUnitsMap.get(unit); - if (mappedUnit != null) { - return metricNameBuilder.append(UNDERSCORE).append(mappedUnit).append(suffix).toString(); - } + if ("1".equals(unit) && isGauge) { + return metricNameBuilder.append(RATIO_SUFFIX).toString(); + } - if (unit.contains("/")) { - String[] unitSplit = unit.split("/", 2); - if (unitSplit.length == 2) { - String unit1 = otelToPrometheusUnitsMap.get(unitSplit[0]); - String unit2 = otelToPrometheusUnitsMap.get(unitSplit[1]); - if (unit1 != null && unit2 != null) { - return metricNameBuilder.append(UNDERSCORE).append(unit1) - .append(UNDERSCORE).append(unit2).append(suffix).toString(); - } + String mappedUnit = otelToPrometheusUnitsMap.get(unit); + if (mappedUnit != null) { + return metricNameBuilder.append(UNDERSCORE).append(mappedUnit).append(suffix).toString(); + } + + if (unit.contains("/")) { + String[] unitSplit = unit.split("/", 2); + if (unitSplit.length == 2) { + String unit1 = otelToPrometheusUnitsMap.get(unitSplit[0]); + String unit2 = otelToPrometheusUnitsMap.get(unitSplit[1]); + if (unit1 != null && unit2 != null) { + return metricNameBuilder.append(UNDERSCORE).append(unit1) + .append(UNDERSCORE).append(unit2).append(suffix).toString(); } } + } - if (!"1".equals(unit)) { - metricNameBuilder.append(UNDERSCORE).append(unit); - } + if (!"1".equals(unit)) { + metricNameBuilder.append(UNDERSCORE).append(unit); } return metricNameBuilder.append(suffix).toString(); }