Skip to content

Commit 5d4095b

Browse files
onurkybsionurkybsi
committed
PR comments addressed
1 parent 9886944 commit 5d4095b

File tree

2 files changed

+58
-67
lines changed

2 files changed

+58
-67
lines changed

‎exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverter.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import java.util.Locale;
6262
import java.util.Map;
6363
import java.util.Set;
64+
import java.util.StringJoiner;
6465
import java.util.concurrent.ConcurrentHashMap;
6566
import java.util.concurrent.TimeUnit;
6667
import java.util.function.Predicate;
@@ -658,23 +659,25 @@ private static String toLabelValue(AttributeType type, Object attributeValue) {
658659
case DOUBLE_ARRAY:
659660
case STRING_ARRAY:
660661
if (attributeValue instanceof List) {
661-
return ((List<?>) attributeValue)
662-
.stream()
663-
.map(String::valueOf)
664-
.map(it -> AttributeType.STRING_ARRAY.equals(type) ? toJsonValidStr(it) : it)
665-
.collect(Collectors.toList())
666-
.toString();
662+
return toJsonStr((List<?>) attributeValue);
667663
} else {
668-
LOGGER.log(
669-
Level.WARNING,
670-
"Unexpected label value of {0} for {1}, toString() is being used as fallback value...",
671-
new Object[] {attributeValue.getClass(), type.name()});
672-
return attributeValue.toString();
664+
throw new IllegalStateException(
665+
String.format(
666+
"Unexpected label value of %s for %s",
667+
attributeValue.getClass().getName(), type.name()));
673668
}
674669
}
675670
throw new IllegalStateException("Unrecognized AttributeType: " + type);
676671
}
677672

673+
public static String toJsonStr(List<?> attributeValue) {
674+
StringJoiner joiner = new StringJoiner(",", "[", "]");
675+
for (Object value : attributeValue) {
676+
joiner.add(value instanceof String ? toJsonValidStr((String) value) : String.valueOf(value));
677+
}
678+
return joiner.toString();
679+
}
680+
678681
public static String toJsonValidStr(String str) {
679682
StringBuilder sb = new StringBuilder();
680683
sb.append('"');

‎exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/Otel2PrometheusConverterTest.java‎

Lines changed: 44 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,19 @@
55

66
package io.opentelemetry.exporter.prometheus;
77

8+
import static io.opentelemetry.api.common.AttributeKey.booleanArrayKey;
9+
import static io.opentelemetry.api.common.AttributeKey.booleanKey;
10+
import static io.opentelemetry.api.common.AttributeKey.doubleArrayKey;
11+
import static io.opentelemetry.api.common.AttributeKey.doubleKey;
12+
import static io.opentelemetry.api.common.AttributeKey.longArrayKey;
13+
import static io.opentelemetry.api.common.AttributeKey.longKey;
14+
import static io.opentelemetry.api.common.AttributeKey.stringArrayKey;
815
import static io.opentelemetry.api.common.AttributeKey.stringKey;
916
import static org.assertj.core.api.Assertions.assertThat;
1017
import static org.assertj.core.api.Assertions.assertThatCode;
1118

1219
import com.fasterxml.jackson.core.JsonProcessingException;
1320
import com.fasterxml.jackson.databind.ObjectMapper;
14-
import io.opentelemetry.api.common.AttributeKey;
1521
import io.opentelemetry.api.common.AttributeType;
1622
import io.opentelemetry.api.common.Attributes;
1723
import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
@@ -32,6 +38,7 @@
3238
import io.opentelemetry.sdk.metrics.internal.data.ImmutableSummaryPointData;
3339
import io.opentelemetry.sdk.resources.Resource;
3440
import io.prometheus.metrics.expositionformats.ExpositionFormats;
41+
import io.prometheus.metrics.model.snapshots.Labels;
3542
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
3643
import java.io.ByteArrayOutputStream;
3744
import java.io.IOException;
@@ -57,6 +64,7 @@ class Otel2PrometheusConverterTest {
5764
private static final Pattern PATTERN =
5865
Pattern.compile(
5966
"# HELP (?<help>.*)\n# TYPE (?<type>.*)\n(?<metricName>.*)\\{otel_scope_name=\"scope\"}(.|\\n)*");
67+
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
6068

6169
private final Otel2PrometheusConverter converter =
6270
new Otel2PrometheusConverter(true, /* allowedResourceAttributesFilter= */ null);
@@ -141,66 +149,28 @@ void prometheusNameCollisionTest_Issue6277() {
141149
assertThatCode(() -> converter.convert(metricData)).doesNotThrowAnyException();
142150
}
143151

144-
@Test
145-
void labelValueSerialization_Primitives() {
146-
Attributes attributes =
147-
Attributes.builder()
148-
.put(AttributeKey.stringKey("stringKey"), "stringValue")
149-
.put(AttributeKey.booleanKey("booleanKey"), true)
150-
.put(AttributeKey.longKey("longKey"), Long.MAX_VALUE)
151-
.put(AttributeKey.doubleKey("doubleKey"), 0.12345)
152-
.build();
153-
MetricData metricData =
154-
createSampleMetricData("sample", "1", MetricDataType.LONG_SUM, attributes, null);
155-
156-
MetricSnapshots snapshots = converter.convert(Collections.singletonList(metricData));
157-
158-
assertThat(snapshots.get(0).getDataPoints().get(0).getLabels().get("stringKey"))
159-
.isEqualTo("stringValue");
160-
assertThat(snapshots.get(0).getDataPoints().get(0).getLabels().get("booleanKey"))
161-
.isEqualTo("true");
162-
assertThat(snapshots.get(0).getDataPoints().get(0).getLabels().get("longKey"))
163-
.isEqualTo("9223372036854775807");
164-
assertThat(snapshots.get(0).getDataPoints().get(0).getLabels().get("doubleKey"))
165-
.isEqualTo("0.12345");
166-
}
167-
168-
@Test
169-
void labelValueSerialization_NonPrimitives() throws JsonProcessingException {
170-
List<String> stringArrayValue =
171-
Arrays.asList("stringValue1", "\"+\\\\\\+\b+\f+\n+\r+\t+" + (char) 0);
172-
List<Boolean> booleanArrayValue = Arrays.asList(true, false);
173-
List<Long> longArrayValue = Arrays.asList(Long.MIN_VALUE, Long.MAX_VALUE);
174-
List<Double> doubleArrayValue = Arrays.asList(Double.MIN_VALUE, Double.MAX_VALUE);
175-
Attributes attributes =
176-
Attributes.builder()
177-
.put(AttributeKey.stringArrayKey("stringKey"), stringArrayValue)
178-
.put(AttributeKey.booleanArrayKey("booleanKey"), booleanArrayValue)
179-
.put(AttributeKey.longArrayKey("longKey"), longArrayValue)
180-
.put(AttributeKey.doubleArrayKey("doubleKey"), doubleArrayValue)
181-
.build();
152+
@ParameterizedTest
153+
@MethodSource("labelValueSerializationArgs")
154+
void labelValueSerialization(Attributes attributes) {
182155
MetricData metricData =
183156
createSampleMetricData("sample", "1", MetricDataType.LONG_SUM, attributes, null);
184157

185158
MetricSnapshots snapshots = converter.convert(Collections.singletonList(metricData));
186159

187-
ObjectMapper objectMapper = new ObjectMapper();
188-
assertThat(
189-
objectMapper.readTree(
190-
snapshots.get(0).getDataPoints().get(0).getLabels().get("stringKey")))
191-
.isEqualTo(objectMapper.valueToTree(stringArrayValue));
192-
assertThat(
193-
objectMapper.readTree(
194-
snapshots.get(0).getDataPoints().get(0).getLabels().get("booleanKey")))
195-
.isEqualTo(objectMapper.valueToTree(booleanArrayValue));
196-
assertThat(
197-
objectMapper.readTree(
198-
snapshots.get(0).getDataPoints().get(0).getLabels().get("longKey")))
199-
.isEqualTo(objectMapper.valueToTree(longArrayValue));
200-
assertThat(
201-
objectMapper.readTree(
202-
snapshots.get(0).getDataPoints().get(0).getLabels().get("doubleKey")))
203-
.isEqualTo(objectMapper.valueToTree(doubleArrayValue));
160+
Labels labels = snapshots.get(0).getDataPoints().get(0).getLabels();
161+
attributes.forEach(
162+
(key, value) -> {
163+
String labelValue = labels.get(key.getKey());
164+
try {
165+
String expectedValue =
166+
key.getType() == AttributeType.STRING
167+
? (String) value
168+
: OBJECT_MAPPER.writeValueAsString(value);
169+
assertThat(labelValue).isEqualTo(expectedValue);
170+
} catch (JsonProcessingException e) {
171+
throw new RuntimeException(e);
172+
}
173+
});
204174
}
205175

206176
@Test
@@ -375,6 +345,24 @@ private static Stream<Arguments> metricMetadataArgs() {
375345
"_metric_name_bytes_count"));
376346
}
377347

348+
private static Stream<Arguments> labelValueSerializationArgs() {
349+
return Stream.of(
350+
Arguments.of(Attributes.of(stringKey("key"), "stringValue")),
351+
Arguments.of(Attributes.of(booleanKey("key"), true)),
352+
Arguments.of(Attributes.of(longKey("key"), Long.MAX_VALUE)),
353+
Arguments.of(Attributes.of(doubleKey("key"), 0.12345)),
354+
Arguments.of(
355+
Attributes.of(
356+
stringArrayKey("key"),
357+
Arrays.asList("stringValue1", "\"+\\\\\\+\b+\f+\n+\r+\t+" + (char) 0))),
358+
Arguments.of(Attributes.of(booleanArrayKey("key"), Arrays.asList(true, false))),
359+
Arguments.of(
360+
Attributes.of(longArrayKey("key"), Arrays.asList(Long.MIN_VALUE, Long.MAX_VALUE))),
361+
Arguments.of(
362+
Attributes.of(
363+
doubleArrayKey("key"), Arrays.asList(Double.MIN_VALUE, Double.MAX_VALUE))));
364+
}
365+
378366
static MetricData createSampleMetricData(
379367
String metricName, Resource resource, Attributes attributes) {
380368
return createSampleMetricData(

0 commit comments

Comments
 (0)