Skip to content

Commit

Permalink
addressing more comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Owen <[email protected]>
  • Loading branch information
@OwenCorrigan76
OwenCorrigan76 committed Jun 17, 2024
1 parent fd67d75 commit 232c363
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 35 deletions.
17 changes: 8 additions & 9 deletions src/main/java/io/strimzi/kafka/metrics/KafkaMetricsCollector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ public List<MetricFamilySamples> collect() {
continue;
}
LOG.info("Kafka metric {} is allowed", name);
LOG.info("labels " + metricName.tags());
MetricFamilySamples sample = convert(name, metricName.description(), kafkaMetric, metricName.tags());
MetricFamilySamples sample = convert(name, metricName.description(), kafkaMetric, metricName.tags(), metricName);
if (sample != null) {
samples.add(sample);
}
Expand Down Expand Up @@ -109,28 +108,28 @@ String metricName(MetricName metricName) {
return prefix + '_' + group + '_' + name;
}

static MetricFamilySamples convert(String name, String help, KafkaMetric metric, Map<String, String> labels) {
Object valueObj = metric.metricValue();
double value;
static MetricFamilySamples convert(String name, String help, KafkaMetric metric, Map<String, String> labels, MetricName metricName) {
Map<String, String> sanitizedLabels = labels.entrySet().stream()
.collect(Collectors.toMap(
e -> Collector.sanitizeMetricName(e.getKey()),
Map.Entry::getValue,
(v1, v2) -> {
throw new IllegalStateException("Unexpected duplicate key " + v1);
LOG.warn("Unexpected duplicate key" + v1);
return v1;
},
LinkedHashMap::new));

Object valueObj = metric.metricValue();
double value;
if (valueObj instanceof Number) {
value = ((Number) valueObj).doubleValue();
} else {
value = 1.0;
sanitizedLabels.put(name, String.valueOf(valueObj));
String attributeName = metricName.name();
sanitizedLabels.put(attributeName, String.valueOf(valueObj));
}

return new MetricFamilySamplesBuilder(Type.GAUGE, help)
.addSample(name, value, sanitizedLabels)
.build();
}

}
15 changes: 8 additions & 7 deletions src/main/java/io/strimzi/kafka/metrics/YammerMetricsCollector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public List<MetricFamilySamples> collect() {
if (metric instanceof Counter) {
sample = convert(name, (Counter) metric, labels);
} else if (metric instanceof Gauge) {
sample = convert(name, (Gauge<?>) metric, labels);
sample = convert(name, (Gauge<?>) metric, labels, metricName);
} else if (metric instanceof Histogram) {
sample = convert(name, (Histogram) metric, labels);
} else if (metric instanceof Meter) {
Expand Down Expand Up @@ -121,23 +121,24 @@ static MetricFamilySamples convert(String name, Counter counter, Map<String, Str
.build();
}

static MetricFamilySamples convert(String name, Gauge<?> gauge, Map<String, String> labels) {
Object valueObj = gauge.value();
double value;
static MetricFamilySamples convert(String name, Gauge<?> gauge, Map<String, String> labels, MetricName metricName) {
Map<String, String> sanitizedLabels = labels.entrySet().stream()
.collect(Collectors.toMap(
e -> Collector.sanitizeMetricName(e.getKey()),
Map.Entry::getValue,
(v1, v2) -> {
throw new IllegalStateException("Unexpected duplicate key " + v1);
LOG.warn("Unexpected duplicate key " + v1);
return v1;
},
LinkedHashMap::new));

Object valueObj = gauge.value();
double value;
if (valueObj instanceof Number) {
value = ((Number) valueObj).doubleValue();
} else {
value = 1.0;
sanitizedLabels.put("value", String.valueOf(valueObj));
String attributeName = metricName.getName();
sanitizedLabels.put(attributeName, String.valueOf(valueObj));
}

return new MetricFamilySamplesBuilder(Type.GAUGE, "")
Expand Down
9 changes: 5 additions & 4 deletions src/test/java/io/strimzi/kafka/metrics/KafkaMetricsCollectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

Expand Down Expand Up @@ -82,8 +83,8 @@ public void testCollectNonNumericMetric() {
collector.addMetric(nonNumericMetric);
metrics = collector.collect();

Map<String, String> expectedLabels = new HashMap<>(labels);
expectedLabels.put("kafka_server_group_name", nonNumericValue);
Map<String, String> expectedLabels = new LinkedHashMap<>(labels);
expectedLabels.put("name", nonNumericValue);
assertEquals(1, metrics.size());

Collector.MetricFamilySamples metricFamilySamples = metrics.get(0);
Expand All @@ -99,7 +100,7 @@ private void assertMetricFamilySample(Collector.MetricFamilySamples actual, Stri

Collector.MetricFamilySamples.Sample actualSample = actual.samples.get(0);

assertEquals(expectedValue, actualSample.value, 0.1, "unexpected value");
assertEquals(actualSample.value, expectedValue, 0.1, "unexpected value");
assertEquals(new ArrayList<>(expectedLabels.keySet()), actualSample.labelNames, "sample has unexpected label names");
assertEquals(new ArrayList<>(expectedLabels.values()), actualSample.labelValues, "sample has unexpected label values");
}
Expand All @@ -124,4 +125,4 @@ private KafkaMetric buildNonNumericMetric(String name, String group, String nonN
time);
}

}
}
24 changes: 9 additions & 15 deletions src/test/java/io/strimzi/kafka/metrics/YammerMetricsCollectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,7 @@
import io.prometheus.client.Collector;
import org.apache.kafka.server.metrics.KafkaYammerMetrics;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.MethodOrderer;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;

import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -24,7 +21,6 @@

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)

public class YammerMetricsCollectorTest {

Expand All @@ -38,7 +34,6 @@ public void setup() {
}

@Test
@Order(1)
public void testMetricLifeCycle() {
Map<String, String> props = new HashMap<>();
props.put(PrometheusMetricsReporterConfig.ALLOWLIST_CONFIG, "kafka_server_group_name.*");
Expand All @@ -59,10 +54,10 @@ public void testMetricLifeCycle() {
assertEquals(1, metrics.size());

Collector.MetricFamilySamples metricFamilySamples = metrics.get(0);
Collector.MetricFamilySamples.Sample serverGroupNameSamples = metricFamilySamples.samples.get(0);

assertEquals("kafka_server_group_name_type_count", metrics.get(0).name);
assertEquals(1, metricFamilySamples.samples.size());

Collector.MetricFamilySamples.Sample serverGroupNameSamples = metricFamilySamples.samples.get(0);
assertEquals(0.0, serverGroupNameSamples.value, 0.1);
assertEquals(new ArrayList<>(tags.keySet()), serverGroupNameSamples.labelNames);
assertEquals(new ArrayList<>(tags.values()), serverGroupNameSamples.labelValues);
Expand All @@ -72,12 +67,12 @@ public void testMetricLifeCycle() {
metrics = collector.collect();
assertEquals(1, metrics.size());

Collector.MetricFamilySamples metricFamilySamples1 = metrics.get(0);
Collector.MetricFamilySamples.Sample serverGroupNameSamples1 = metricFamilySamples1.samples.get(0);
metricFamilySamples = metrics.get(0);
serverGroupNameSamples = metricFamilySamples.samples.get(0);

assertEquals("kafka_server_group_name_type_count", metricFamilySamples1.name);
assertEquals(1, metricFamilySamples1.samples.size());
assertEquals(10.0, serverGroupNameSamples1.value, 0.1);
assertEquals("kafka_server_group_name_type_count", metricFamilySamples.name);
assertEquals(1, metricFamilySamples.samples.size());
assertEquals(10.0, serverGroupNameSamples.value, 0.1);

// Removing the metric
removeMetric("group", "name", "type");
Expand All @@ -86,7 +81,6 @@ public void testMetricLifeCycle() {
}

@Test
@Order(2)
public void testCollectNonNumericMetric() {
Map<String, String> props = new HashMap<>();
props.put(PrometheusMetricsReporterConfig.ALLOWLIST_CONFIG, "kafka_server_group_name.*");
Expand All @@ -100,7 +94,7 @@ public void testCollectNonNumericMetric() {
metrics = collector.collect();

Map<String, String> expectedTags = new LinkedHashMap<>(tags);
expectedTags.put("value", "value");
expectedTags.put("type", "value");
assertEquals(1, metrics.size());

Collector.MetricFamilySamples metricFamilySamples = metrics.get(0);
Expand Down Expand Up @@ -150,4 +144,4 @@ public void removeMetric(String group, String name, String type) {
KafkaYammerMetrics.defaultRegistry().removeMetric(metricName);
}

}
}

0 comments on commit 232c363

Please sign in to comment.