Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning log when duplicate leables are created or detected #29

Merged
merged 6 commits into from
Jul 16, 2024
Merged

Add warning log when duplicate leables are created or detected #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c25469d
duplicate key warning
OwenCorrigan76 Jul 8, 2024
7ede3d8
duplicate key warning and tests
OwenCorrigan76 Jul 10, 2024
8f3c7f7
refactor to address comments
OwenCorrigan76 Jul 12, 2024
3660769
test non-equal labels
OwenCorrigan76 Jul 12, 2024
b24f143
fix checkstyle
OwenCorrigan76 Jul 12, 2024
5704539
remove unwanted empty line
OwenCorrigan76 Jul 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/main/java/io/strimzi/kafka/metrics/DataPointSnapshotBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,16 @@
import io.prometheus.metrics.model.snapshots.PrometheusNaming;
import io.prometheus.metrics.model.snapshots.Quantiles;
import io.prometheus.metrics.model.snapshots.SummarySnapshot;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Builder methods to convert Kafka metrics into Prometheus metrics
*/
public class DataPointSnapshotBuilder {

private static final Logger LOG = LoggerFactory.getLogger(DataPointSnapshotBuilder.class.getName());

/**
* Create a datapoint for a {@link InfoSnapshot} metric
* @param labels The labels associated with the datapoint
Expand All @@ -25,7 +29,17 @@ public class DataPointSnapshotBuilder {
* @return The {@link InfoSnapshot.InfoDataPointSnapshot} datapoint
*/
public static InfoSnapshot.InfoDataPointSnapshot infoDataPoint(Labels labels, Object value, String metricName) {
Labels newLabels = labels.add(PrometheusNaming.sanitizeLabelName(metricName), String.valueOf(value));
String newLabelName = PrometheusNaming.sanitizeLabelName(metricName);
Labels newLabels = labels;
String existingValue = labels.get(newLabelName);
if (existingValue != null) {
if (!String.valueOf(value).equals(existingValue)) {
LOG.warn("Unable to add new label because of duplicate key: {} with value: {} from metric: {}",
newLabelName, value, metricName);
}
} else {
newLabels = labels.add(newLabelName, String.valueOf(value));
}
return InfoSnapshot.InfoDataPointSnapshot.builder()
.labels(newLabels)
.build();
Expand Down
14 changes: 11 additions & 3 deletions src/main/java/io/strimzi/kafka/metrics/KafkaMetricsCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@

import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

/**
Expand Down Expand Up @@ -94,7 +96,7 @@ public MetricSnapshots collect() {
continue;
}
LOG.info("Kafka metric {} is allowed", prometheusMetricName);
Labels labels = labelsFromTags(metricName.tags());
Labels labels = labelsFromTags(metricName.tags(), metricName.name());

Object valueObj = kafkaMetric.metricValue();
if (valueObj instanceof Number) {
Expand Down Expand Up @@ -122,10 +124,16 @@ private String metricName(MetricName metricName) {
.toLowerCase(Locale.ROOT);
}

private static Labels labelsFromTags(Map<String, String> tags) {
static Labels labelsFromTags(Map<String, String> tags, String metricName) {
Labels.Builder builder = Labels.builder();
Set<String> labelNames = new HashSet<>();
for (Map.Entry<String, String> label : tags.entrySet()) {
builder.label(PrometheusNaming.sanitizeLabelName(label.getKey()), label.getValue());
String newLabelName = PrometheusNaming.sanitizeLabelName(label.getKey());
if (labelNames.add(newLabelName)) {
builder.label(newLabelName, label.getValue());
} else {
LOG.warn("Ignoring duplicate label key: {} with value: {} from metric: {} ", newLabelName, label.getValue(), metricName);
}
}
return builder.build();
}
Expand Down
15 changes: 12 additions & 3 deletions src/main/java/io/strimzi/kafka/metrics/YammerMetricsCollector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;

/**
* Prometheus Collector to store and export metrics retrieved by {@link YammerPrometheusMetricsReporter}.
Expand Down Expand Up @@ -83,7 +85,7 @@ public MetricSnapshots collect() {
continue;
}
LOG.info("Yammer metric {} is allowed", prometheusMetricName);
Labels labels = labelsFromScope(metricName.getScope());
Labels labels = labelsFromScope(metricName.getScope(), prometheusMetricName);
LOG.info("labels {}", labels);

if (metric instanceof Counter) {
Expand Down Expand Up @@ -143,13 +145,20 @@ private static String metricName(MetricName metricName) {
return metricNameStr;
}

static Labels labelsFromScope(String scope) {
static Labels labelsFromScope(String scope, String metricName) {
Labels.Builder builder = Labels.builder();
Set<String> labelNames = new HashSet<>();
if (scope != null) {
String[] parts = scope.split("\\.");
if (parts.length % 2 == 0) {
for (int i = 0; i < parts.length; i += 2) {
builder.label(PrometheusNaming.sanitizeLabelName(parts[i]), parts[i + 1]);
String newLabelName = PrometheusNaming.sanitizeLabelName(parts[i]);
if (labelNames.add(newLabelName)) {
builder.label(newLabelName, parts[i + 1]);
} else {
String value = parts[i + 1];
LOG.warn("Ignoring duplicate label key: {} with value: {} from metric: {} ", newLabelName, value, metricName);
}
}
}
}
Expand Down
22 changes: 22 additions & 0 deletions src/test/java/io/strimzi/kafka/metrics/DataPointSnapshotBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/*
* Copyright Strimzi authors.
* License: Apache License 2.0 (see the file LICENSE or http://apache.org/licenses/LICENSE-2.0.html).
*/
package io.strimzi.kafka.metrics;

import io.prometheus.metrics.model.snapshots.InfoSnapshot;
import io.prometheus.metrics.model.snapshots.Labels;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class DataPointSnapshotBuilderTest {

@Test
public void testCollidingNewLabelIsIgnored() {
Labels labels = Labels.builder().label("k_1", "v1").label("k2", "v2").build();
InfoSnapshot.InfoDataPointSnapshot snapshot = DataPointSnapshotBuilder.infoDataPoint(labels, "value", "k-1");
assertEquals("v1", snapshot.getLabels().get("k_1"));
}

}
14 changes: 14 additions & 0 deletions src/test/java/io/strimzi/kafka/metrics/KafkaMetricsCollectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.prometheus.metrics.model.snapshots.Labels;
import io.prometheus.metrics.model.snapshots.MetricSnapshot;
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
import io.prometheus.metrics.model.snapshots.PrometheusNaming;
import org.apache.kafka.common.MetricName;
import org.apache.kafka.common.metrics.Gauge;
import org.apache.kafka.common.metrics.KafkaMetric;
Expand Down Expand Up @@ -108,6 +109,19 @@ public void testCollectNonNumericMetric() {
assertEquals(expectedLabels, snapshot.getDataPoints().get(0).getLabels());
}

@Test
public void testLabelsFromTags() {
Map<String, String> tags = new LinkedHashMap<>();
tags.put("k-1", "v1");
tags.put("k_1", "v2");

Labels labels = KafkaMetricsCollector.labelsFromTags(tags, "name");

assertEquals("k_1", PrometheusNaming.sanitizeLabelName("k-1"));
assertEquals("v1", labels.get("k_1"));
assertEquals(1, labels.size());
}

private void assertGaugeSnapshot(MetricSnapshot snapshot, double expectedValue, Labels expectedLabels) {
assertInstanceOf(GaugeSnapshot.class, snapshot);
GaugeSnapshot gaugeSnapshot = (GaugeSnapshot) snapshot;
Expand Down
16 changes: 11 additions & 5 deletions src/test/java/io/strimzi/kafka/metrics/YammerMetricsCollectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import io.prometheus.metrics.model.snapshots.Labels;
import io.prometheus.metrics.model.snapshots.MetricSnapshot;
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
import io.prometheus.metrics.model.snapshots.PrometheusNaming;
import org.apache.kafka.server.metrics.KafkaYammerMetrics;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -115,11 +116,16 @@ public void testCollectNonNumericMetric() {

@Test
public void testLabelsFromScope() {
assertEquals(Labels.of("k1", "v1", "k2", "v2"), YammerMetricsCollector.labelsFromScope("k1.v1.k2.v2"));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope(null));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope("k1"));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope("k1."));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope("k1.v1.k"));
assertEquals(Labels.of("k1", "v1", "k2", "v2"), YammerMetricsCollector.labelsFromScope("k1.v1.k2.v2", "name"));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope(null, "name"));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope("k1", "name"));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope("k1.", "name"));
assertEquals(Labels.EMPTY, YammerMetricsCollector.labelsFromScope("k1.v1.k", "name"));

Labels labels = YammerMetricsCollector.labelsFromScope("k-1.v1.k_1.v2", "name");
assertEquals("k_1", PrometheusNaming.sanitizeLabelName("k-1"));
assertEquals("v1", labels.get("k_1"));
assertEquals(1, labels.size());
}

public Counter newCounter(String group, String type, String name) {
Expand Down