Skip to content

Commit caa42bf

Browse files
committed
Add "get[Low|High]CardinalityKeyValue()" on "Observation.Context"
Add two new methods `getLowCardinalityKeyValue` and `getHighCardinalityKeyValue`. Also, changes the internal holder from `Set` to `Map`. Prior to this change, the usage of `LinkedHashSet` allowed multiple `KeyValue`s with the same key. It was deduped when converted to `KeyValues`, but it is not an ideal implementation. This change replaces the `LinkedHashSet` with `LinkedHashMap` using the key from `KeyValue` as a map key. This way, when a new `KeyValue` is added, it will override the old `KeyValue` with the same key.
1 parent c574021 commit caa42bf

File tree

3 files changed

+79
-7
lines changed

3 files changed

+79
-7
lines changed

micrometer-observation/src/main/java/io/micrometer/observation/Observation.java

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@
2020
import io.micrometer.common.lang.NonNull;
2121
import io.micrometer.common.lang.Nullable;
2222

23-
import java.util.*;
23+
import java.util.HashMap;
24+
import java.util.LinkedHashMap;
25+
import java.util.Map;
2426
import java.util.concurrent.Callable;
2527
import java.util.function.Function;
2628
import java.util.function.Supplier;
@@ -751,9 +753,9 @@ class Context implements ContextView {
751753
@Nullable
752754
private ObservationView parentObservation;
753755

754-
private final Set<KeyValue> lowCardinalityKeyValues = new LinkedHashSet<>();
756+
private final Map<String, KeyValue> lowCardinalityKeyValues = new LinkedHashMap<>();
755757

756-
private final Set<KeyValue> highCardinalityKeyValues = new LinkedHashSet<>();
758+
private final Map<String, KeyValue> highCardinalityKeyValues = new LinkedHashMap<>();
757759

758760
/**
759761
* The observation name.
@@ -923,7 +925,7 @@ public void clear() {
923925
* @return this context
924926
*/
925927
public Context addLowCardinalityKeyValue(KeyValue keyValue) {
926-
this.lowCardinalityKeyValues.add(keyValue);
928+
this.lowCardinalityKeyValues.put(keyValue.getKey(), keyValue);
927929
return this;
928930
}
929931

@@ -935,7 +937,7 @@ public Context addLowCardinalityKeyValue(KeyValue keyValue) {
935937
* @return this context
936938
*/
937939
public Context addHighCardinalityKeyValue(KeyValue keyValue) {
938-
this.highCardinalityKeyValues.add(keyValue);
940+
this.highCardinalityKeyValues.put(keyValue.getKey(), keyValue);
939941
return this;
940942
}
941943

@@ -962,13 +964,23 @@ public Context addHighCardinalityKeyValues(KeyValues keyValues) {
962964
@NonNull
963965
@Override
964966
public KeyValues getLowCardinalityKeyValues() {
965-
return KeyValues.of(this.lowCardinalityKeyValues);
967+
return KeyValues.of(this.lowCardinalityKeyValues.values());
966968
}
967969

968970
@NonNull
969971
@Override
970972
public KeyValues getHighCardinalityKeyValues() {
971-
return KeyValues.of(this.highCardinalityKeyValues);
973+
return KeyValues.of(this.highCardinalityKeyValues.values());
974+
}
975+
976+
@Override
977+
public KeyValue getLowCardinalityKeyValue(String key) {
978+
return this.lowCardinalityKeyValues.get(key);
979+
}
980+
981+
@Override
982+
public KeyValue getHighCardinalityKeyValue(String key) {
983+
return this.highCardinalityKeyValues.get(key);
972984
}
973985

974986
@NonNull
@@ -1149,6 +1161,22 @@ interface ContextView {
11491161
@NonNull
11501162
KeyValues getHighCardinalityKeyValues();
11511163

1164+
/**
1165+
* Returns a low cardinality key value or {@code null} if not present.
1166+
* @param key key
1167+
* @return a low cardinality key value or {@code null}
1168+
*/
1169+
@Nullable
1170+
KeyValue getLowCardinalityKeyValue(String key);
1171+
1172+
/**
1173+
* Returns a high cardinality key value or {@code null} if not present.
1174+
* @param key key
1175+
* @return a high cardinality key value or {@code null}
1176+
*/
1177+
@Nullable
1178+
KeyValue getHighCardinalityKeyValue(String key);
1179+
11521180
/**
11531181
* Returns all key values.
11541182
* @return all key values

micrometer-observation/src/test/java/io/micrometer/observation/ObservationContextTest.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.micrometer.observation;
1717

18+
import io.micrometer.common.KeyValue;
1819
import org.assertj.core.api.Assertions;
1920
import org.junit.jupiter.api.BeforeEach;
2021
import org.junit.jupiter.api.Test;
@@ -109,4 +110,25 @@ void cleanEmptyContextShouldNotFail() {
109110
context.clear();
110111
}
111112

113+
@Test
114+
void sameKeyShouldOverrideKeyValue() {
115+
KeyValue low = KeyValue.of("low", "LOW");
116+
KeyValue high = KeyValue.of("high", "HIGH");
117+
context.addLowCardinalityKeyValue(low);
118+
context.addHighCardinalityKeyValue(high);
119+
120+
assertThat(context.getLowCardinalityKeyValue("low")).isSameAs(low);
121+
assertThat(context.getHighCardinalityKeyValue("high")).isSameAs(high);
122+
123+
KeyValue newLow = KeyValue.of("low", "LOW-NEW");
124+
KeyValue newHigh = KeyValue.of("high", "HIGH-NEW");
125+
context.addLowCardinalityKeyValue(newLow);
126+
context.addHighCardinalityKeyValue(newHigh);
127+
128+
assertThat(context.getLowCardinalityKeyValue("low")).isSameAs(newLow);
129+
assertThat(context.getHighCardinalityKeyValue("high")).isSameAs(newHigh);
130+
assertThat(context.getLowCardinalityKeyValues()).containsExactly(newLow);
131+
assertThat(context.getHighCardinalityKeyValues()).containsExactly(newHigh);
132+
}
133+
112134
}

micrometer-observation/src/test/java/io/micrometer/observation/ObservationTests.java

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.micrometer.observation;
1717

18+
import io.micrometer.common.KeyValue;
1819
import org.junit.jupiter.api.Test;
1920

2021
import java.io.IOException;
@@ -78,6 +79,27 @@ void havingAnObservationFilterWillMutateTheContext() {
7879
assertThat((String) context.get("foo")).isEqualTo("bar");
7980
}
8081

82+
@Test
83+
void havingAnObservationFilterToModifyKeyValue() {
84+
registry.observationConfig().observationHandler(context -> true);
85+
86+
Observation.Context context = new Observation.Context();
87+
context.addHighCardinalityKeyValue(KeyValue.of("foo", "FOO"));
88+
89+
ObservationFilter filter = (ctx) -> {
90+
KeyValue keyValue = ctx.getHighCardinalityKeyValue("foo");
91+
assertThat(keyValue).isNotNull();
92+
return ctx.addHighCardinalityKeyValue(KeyValue.of(keyValue.getKey(), keyValue.getValue() + "-modified"));
93+
};
94+
95+
registry.observationConfig().observationFilter(filter);
96+
97+
Observation.start("foo", () -> context, registry).stop();
98+
99+
assertThat(context.getHighCardinalityKeyValue("foo")).isNotNull().extracting(KeyValue::getValue)
100+
.isEqualTo("FOO-modified");
101+
}
102+
81103
@Test
82104
void settingParentObservationMakesAReferenceOnParentContext() {
83105
registry.observationConfig().observationHandler(context -> true);

0 commit comments

Comments
 (0)