Skip to content

Commit 4854095

Browse files
lauritlaurit
committed
address review comments
1 parent c0076c2 commit 4854095

File tree

2 files changed

+144
-97
lines changed
  • instrumentation/logback/logback-appender-1.0/library/src

2 files changed

+144
-97
lines changed

‎instrumentation/logback/logback-appender-1.0/library/src/main/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapper.java‎

Lines changed: 93 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.opentelemetry.semconv.ExceptionAttributes;
2424
import java.io.PrintWriter;
2525
import java.io.StringWriter;
26+
import java.lang.reflect.Array;
2627
import java.lang.reflect.Field;
2728
import java.util.ArrayList;
2829
import java.util.Arrays;
@@ -32,7 +33,6 @@
3233
import java.util.Map;
3334
import java.util.concurrent.TimeUnit;
3435
import java.util.function.Function;
35-
import java.util.function.IntFunction;
3636
import java.util.stream.Collectors;
3737
import javax.annotation.Nullable;
3838
import net.logstash.logback.marker.LogstashMarker;
@@ -70,7 +70,13 @@ public final class LoggingEventMapper {
7070
private static final AttributeKey<List<String>> LOG_BODY_PARAMETERS =
7171
AttributeKey.stringArrayKey("log.body.parameters");
7272

73-
private static final Cache<Class<?>, Field> valueField = Cache.bounded(20);
73+
private static final ClassValue<FieldReader> valueField =
74+
new ClassValue<FieldReader>() {
75+
@Override
76+
protected FieldReader computeValue(Class<?> type) {
77+
return createFieldReader(type);
78+
}
79+
};
7480

7581
private final boolean captureExperimentalAttributes;
7682
private final List<String> captureMdcAttributes;
@@ -288,13 +294,13 @@ private static void captureKeyValuePairAttributes(
288294
List<KeyValuePair> keyValuePairs = loggingEvent.getKeyValuePairs();
289295
if (keyValuePairs != null) {
290296
for (KeyValuePair keyValuePair : keyValuePairs) {
291-
captureKeyValueAttribute(attributes, keyValuePair.key, keyValuePair.value);
297+
captureAttribute(attributes, keyValuePair.key, keyValuePair.value);
292298
}
293299
}
294300
}
295301

296-
private static void captureKeyValueAttribute(
297-
AttributesBuilder attributes, Object key, Object value) {
302+
// visible for testing
303+
static void captureAttribute(AttributesBuilder attributes, Object key, Object value) {
298304
// empty values are not serialized
299305
if (key != null && value != null) {
300306
String keyStr = key.toString();
@@ -310,12 +316,8 @@ private static void captureKeyValueAttribute(
310316
attributes.put(keyStr, ((Number) value).doubleValue());
311317
} else if (value.getClass().isArray()) {
312318
if (value instanceof boolean[] || value instanceof Boolean[]) {
313-
captureKeyArrayValueAttribute(
314-
attributes,
315-
AttributeKey.booleanArrayKey(keyStr),
316-
value,
317-
Boolean[]::new,
318-
o -> (Boolean) o);
319+
captureArrayValueAttribute(
320+
attributes, AttributeKey.booleanArrayKey(keyStr), value, o -> (Boolean) o);
319321
} else if (value instanceof byte[]
320322
|| value instanceof Byte[]
321323
|| value instanceof int[]
@@ -324,66 +326,49 @@ private static void captureKeyValueAttribute(
324326
|| value instanceof Long[]
325327
|| value instanceof short[]
326328
|| value instanceof Short[]) {
327-
captureKeyArrayValueAttribute(
328-
attributes,
329-
AttributeKey.longArrayKey(keyStr),
330-
value,
331-
Long[]::new,
332-
o -> ((Number) o).longValue());
329+
captureArrayValueAttribute(
330+
attributes, AttributeKey.longArrayKey(keyStr), value, o -> ((Number) o).longValue());
333331
} else if (value instanceof float[]
334332
|| value instanceof Float[]
335333
|| value instanceof double[]
336334
|| value instanceof Double[]) {
337-
captureKeyArrayValueAttribute(
335+
captureArrayValueAttribute(
338336
attributes,
339337
AttributeKey.doubleArrayKey(keyStr),
340338
value,
341-
Double[]::new,
342339
o -> ((Number) o).doubleValue());
343340
} else {
344-
captureKeyArrayValueAttribute(
345-
attributes,
346-
AttributeKey.stringArrayKey(keyStr),
347-
value,
348-
String[]::new,
349-
String::valueOf);
341+
captureArrayValueAttribute(
342+
attributes, AttributeKey.stringArrayKey(keyStr), value, String::valueOf);
350343
}
351344
} else if (value instanceof Collection) {
352-
captureKeyArrayValueAttribute(
345+
captureArrayValueAttribute(
353346
attributes,
354347
AttributeKey.stringArrayKey(keyStr),
355348
((Collection<?>) value).toArray(),
356-
String[]::new,
357349
String::valueOf);
358350
} else {
359351
attributes.put(getAttributeKey(keyStr), String.valueOf(value));
360352
}
361353
}
362354
}
363355

364-
private static <T> void captureKeyArrayValueAttribute(
356+
private static <T> void captureArrayValueAttribute(
365357
AttributesBuilder attributes,
366358
AttributeKey<List<T>> key,
367359
Object array,
368-
IntFunction<T[]> newArray,
369360
Function<Object, T> extractor) {
370-
int length = java.lang.reflect.Array.getLength(array);
371-
T[] typedArray = newArray.apply(length);
372-
int offset = 0;
361+
List<T> list = new ArrayList<>();
362+
int length = Array.getLength(array);
373363
for (int i = 0; i < length; i++) {
374-
Object value = java.lang.reflect.Array.get(array, i);
375-
// empty values are not serialized
364+
Object value = Array.get(array, i);
376365
if (value != null) {
377-
typedArray[i - offset] = extractor.apply(value);
378-
} else {
379-
offset++;
366+
list.add(extractor.apply(value));
380367
}
381368
}
382369
// empty lists are not serialized
383-
if (length != offset) {
384-
attributes.put(
385-
key,
386-
Arrays.asList(offset == 0 ? typedArray : Arrays.copyOf(typedArray, length - offset)));
370+
if (!list.isEmpty()) {
371+
attributes.put(key, list);
387372
}
388373
}
389374

@@ -520,67 +505,84 @@ private static void captureLogstashMarker(
520505
@NoMuzzle
521506
private static void captureLogstashMarkerAttributes(
522507
AttributesBuilder attributes, LogstashMarker logstashMarker) {
523-
if (logstashMarker instanceof SingleFieldAppendingMarker) {
524-
SingleFieldAppendingMarker singleFieldAppendingMarker =
525-
(SingleFieldAppendingMarker) logstashMarker;
526-
String fieldName = singleFieldAppendingMarker.getFieldName();
527-
Object fieldValue = extractFieldValue(singleFieldAppendingMarker);
528-
captureKeyValueAttribute(attributes, fieldName, fieldValue);
529-
} else if (logstashMarker instanceof MapEntriesAppendingMarker) {
530-
MapEntriesAppendingMarker mapEntriesAppendingMarker =
531-
(MapEntriesAppendingMarker) logstashMarker;
532-
Map<?, ?> map = extractMapValue(mapEntriesAppendingMarker);
533-
if (map != null) {
534-
for (Map.Entry<?, ?> entry : map.entrySet()) {
535-
Object key = entry.getKey();
536-
Object value = entry.getValue();
537-
captureKeyValueAttribute(attributes, key, value);
538-
}
539-
}
508+
FieldReader fieldReader = valueField.get(logstashMarker.getClass());
509+
if (fieldReader != null) {
510+
fieldReader.read(attributes, logstashMarker);
540511
}
541512
}
542513

543-
@Nullable
544-
private static Object extractFieldValue(SingleFieldAppendingMarker singleFieldAppendingMarker) {
545-
// ObjectAppendingMarker.fieldValue since v7.0
546-
// ObjectAppendingMarker.object since v3.0
547-
// RawJsonAppendingMarker.rawJson since v3.0
548-
Field field =
549-
valueField.computeIfAbsent(
550-
singleFieldAppendingMarker.getClass(),
551-
clazz -> findValueField(clazz, new String[] {"fieldValue", "object", "rawJson"}));
552-
if (field != null) {
553-
try {
554-
return field.get(singleFieldAppendingMarker);
555-
} catch (IllegalAccessException e) {
556-
// ignore
557-
}
514+
@NoMuzzle
515+
private static boolean isSingleFieldAppendingMarker(Class<?> type) {
516+
return SingleFieldAppendingMarker.class.isAssignableFrom(type);
517+
}
518+
519+
@NoMuzzle
520+
private static boolean isMapEntriesAppendingMarker(Class<?> type) {
521+
return MapEntriesAppendingMarker.class.isAssignableFrom(type);
522+
}
523+
524+
private static FieldReader createFieldReader(Class<?> type) {
525+
if (isSingleFieldAppendingMarker(type)) {
526+
// ObjectAppendingMarker.fieldValue since v7.0
527+
// ObjectAppendingMarker.object since v3.0
528+
// RawJsonAppendingMarker.rawJson since v3.0
529+
return createStringReader(findValueField(type, "fieldValue", "object", "rawJson"));
530+
} else if (isMapEntriesAppendingMarker(type)) {
531+
// MapEntriesAppendingMarker.map since v3.0
532+
return createMapReader(findValueField(type, "map"));
558533
}
559534
return null;
560535
}
561536

537+
@NoMuzzle
538+
private static String getSingleFieldAppendingMarkerName(Object logstashMarker) {
539+
SingleFieldAppendingMarker singleFieldAppendingMarker =
540+
(SingleFieldAppendingMarker) logstashMarker;
541+
return singleFieldAppendingMarker.getFieldName();
542+
}
543+
562544
@Nullable
563-
private static Map<?, ?> extractMapValue(MapEntriesAppendingMarker mapEntriesAppendingMarker) {
564-
// MapEntriesAppendingMarker.map since v3.0
565-
Field field =
566-
valueField.computeIfAbsent(
567-
mapEntriesAppendingMarker.getClass(),
568-
clazz -> findValueField(clazz, new String[] {"map"}));
569-
if (field != null) {
570-
try {
571-
Object value = field.get(mapEntriesAppendingMarker);
572-
if (value instanceof Map) {
573-
return (Map<?, ?>) value;
545+
private static FieldReader createStringReader(Field field) {
546+
if (field == null) {
547+
return null;
548+
}
549+
return (attributes, logstashMarker) -> {
550+
String fieldName = getSingleFieldAppendingMarkerName(logstashMarker);
551+
Object fieldValue = extractFieldValue(field, logstashMarker);
552+
captureAttribute(attributes, fieldName, fieldValue);
553+
};
554+
}
555+
556+
@Nullable
557+
private static FieldReader createMapReader(Field field) {
558+
if (field == null) {
559+
return null;
560+
}
561+
return (attributes, logstashMarker) -> {
562+
Object fieldValue = extractFieldValue(field, logstashMarker);
563+
if (fieldValue instanceof Map) {
564+
Map<?, ?> map = (Map<?, ?>) fieldValue;
565+
for (Map.Entry<?, ?> entry : map.entrySet()) {
566+
Object key = entry.getKey();
567+
Object value = entry.getValue();
568+
captureAttribute(attributes, key, value);
574569
}
575-
} catch (IllegalAccessException e) {
576-
// ignore
577570
}
571+
};
572+
}
573+
574+
@Nullable
575+
private static Object extractFieldValue(Field field, Object logstashMarker) {
576+
try {
577+
return field.get(logstashMarker);
578+
} catch (IllegalAccessException e) {
579+
// ignore
578580
}
579581
return null;
580582
}
581583

582584
@Nullable
583-
private static Field findValueField(Class<?> clazz, String[] fieldNames) {
585+
private static Field findValueField(Class<?> clazz, String... fieldNames) {
584586
for (String fieldName : fieldNames) {
585587
try {
586588
Field field = clazz.getDeclaredField(fieldName);
@@ -596,17 +598,7 @@ private static Field findValueField(Class<?> clazz, String[] fieldNames) {
596598
private static boolean supportsLogstashMarkers() {
597599
try {
598600
Class.forName("net.logstash.logback.marker.LogstashMarker");
599-
} catch (ClassNotFoundException e) {
600-
return false;
601-
}
602-
603-
try {
604601
Class.forName("net.logstash.logback.marker.SingleFieldAppendingMarker");
605-
} catch (ClassNotFoundException e) {
606-
return false;
607-
}
608-
609-
try {
610602
Class.forName("net.logstash.logback.marker.MapEntriesAppendingMarker");
611603
} catch (ClassNotFoundException e) {
612604
return false;
@@ -615,6 +607,10 @@ private static boolean supportsLogstashMarkers() {
615607
return true;
616608
}
617609

610+
private interface FieldReader {
611+
void read(AttributesBuilder attributes, LogstashMarker logstashMarker);
612+
}
613+
618614
/**
619615
* This class is internal and is hence not for public use. Its APIs are unstable and can change at
620616
* any time.

‎instrumentation/logback/logback-appender-1.0/library/src/test/java/io/opentelemetry/instrumentation/logback/appender/v1_0/internal/LoggingEventMapperTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import io.opentelemetry.api.common.AttributeKey;
1313
import io.opentelemetry.api.common.Attributes;
1414
import io.opentelemetry.api.common.AttributesBuilder;
15+
import java.util.Collections;
1516
import java.util.HashMap;
1617
import java.util.Map;
1718
import org.junit.jupiter.api.Test;
@@ -70,4 +71,54 @@ void testAll() {
7071
entry(AttributeKey.stringKey("key1"), "value1"),
7172
entry(AttributeKey.stringKey("key2"), "value2"));
7273
}
74+
75+
@Test
76+
void testCaptureAttributeArray() {
77+
AttributesBuilder builder = Attributes.builder();
78+
79+
LoggingEventMapper.captureAttribute(builder, "booleanArray", new boolean[] {true});
80+
LoggingEventMapper.captureAttribute(builder, "BooleanArray", new Boolean[] {true});
81+
82+
LoggingEventMapper.captureAttribute(builder, "byteArray", new byte[] {2});
83+
LoggingEventMapper.captureAttribute(builder, "ByteArray", new Byte[] {2});
84+
85+
LoggingEventMapper.captureAttribute(builder, "shortArray", new short[] {2});
86+
LoggingEventMapper.captureAttribute(builder, "ShortArray", new Short[] {2});
87+
88+
LoggingEventMapper.captureAttribute(builder, "intArray", new int[] {2});
89+
LoggingEventMapper.captureAttribute(builder, "IntegerArray", new Integer[] {2});
90+
91+
LoggingEventMapper.captureAttribute(builder, "longArray", new long[] {2});
92+
LoggingEventMapper.captureAttribute(builder, "LongArray", new Long[] {2L});
93+
94+
LoggingEventMapper.captureAttribute(builder, "floatArray", new float[] {2.0f});
95+
LoggingEventMapper.captureAttribute(builder, "FloatArray", new Float[] {2.0f});
96+
97+
LoggingEventMapper.captureAttribute(builder, "doubleArray", new double[] {2.0});
98+
LoggingEventMapper.captureAttribute(builder, "DoubleArray", new Double[] {2.0});
99+
100+
LoggingEventMapper.captureAttribute(builder, "ObjectArray", new Object[] {"test"});
101+
LoggingEventMapper.captureAttribute(builder, "List", Collections.singletonList("test"));
102+
LoggingEventMapper.captureAttribute(builder, "Set", Collections.singleton("test"));
103+
104+
assertThat(builder.build())
105+
.containsOnly(
106+
entry(AttributeKey.booleanArrayKey("booleanArray"), singletonList(Boolean.TRUE)),
107+
entry(AttributeKey.booleanArrayKey("BooleanArray"), singletonList(Boolean.TRUE)),
108+
entry(AttributeKey.longArrayKey("byteArray"), singletonList(2L)),
109+
entry(AttributeKey.longArrayKey("ByteArray"), singletonList(2L)),
110+
entry(AttributeKey.longArrayKey("shortArray"), singletonList(2L)),
111+
entry(AttributeKey.longArrayKey("ShortArray"), singletonList(2L)),
112+
entry(AttributeKey.longArrayKey("intArray"), singletonList(2L)),
113+
entry(AttributeKey.longArrayKey("IntegerArray"), singletonList(2L)),
114+
entry(AttributeKey.longArrayKey("longArray"), singletonList(2L)),
115+
entry(AttributeKey.longArrayKey("LongArray"), singletonList(2L)),
116+
entry(AttributeKey.doubleArrayKey("floatArray"), singletonList(2.0)),
117+
entry(AttributeKey.doubleArrayKey("FloatArray"), singletonList(2.0)),
118+
entry(AttributeKey.doubleArrayKey("doubleArray"), singletonList(2.0)),
119+
entry(AttributeKey.doubleArrayKey("DoubleArray"), singletonList(2.0)),
120+
entry(AttributeKey.stringArrayKey("ObjectArray"), singletonList("test")),
121+
entry(AttributeKey.stringArrayKey("List"), singletonList("test")),
122+
entry(AttributeKey.stringArrayKey("Set"), singletonList("test")));
123+
}
73124
}

0 commit comments

Comments
 (0)