Skip to content

Commit f6c856c

Browse files
famaridonfamaridon
committed
make High Cardinality Key Values optional
Signed-off-by: famaridon <[email protected]>
1 parent 02f7ab3 commit f6c856c

File tree

5 files changed

+79
-69
lines changed

5 files changed

+79
-69
lines changed

Diff for: ‎micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/mail/DefaultMailSendObservationConvention.java

+12-5
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,14 @@
1515
*/
1616
package io.micrometer.jakarta9.instrument.mail;
1717

18+
import io.micrometer.common.KeyValue;
1819
import io.micrometer.common.KeyValues;
1920
import jakarta.mail.Message;
2021
import jakarta.mail.Message.RecipientType;
2122

23+
import java.util.ArrayList;
24+
import java.util.List;
25+
2226
/**
2327
* Default implementation for {@link MailSendObservationConvention}.
2428
*
@@ -45,11 +49,14 @@ public KeyValues getLowCardinalityKeyValues(MailSendObservationContext context)
4549
@Override
4650
public KeyValues getHighCardinalityKeyValues(MailSendObservationContext context) {
4751
Message message = context.getCarrier();
48-
return KeyValues.of(MailKeyValues.smtpMessageFrom(message),
49-
MailKeyValues.smtpMessageRecipients(message, RecipientType.TO),
50-
MailKeyValues.smtpMessageRecipients(message, RecipientType.CC),
51-
MailKeyValues.smtpMessageRecipients(message, RecipientType.BCC),
52-
MailKeyValues.smtpMessageSubject(message));
52+
List<KeyValue> values = new ArrayList<>();
53+
MailKeyValues.smtpMessageSubject(message).ifPresent(values::add);
54+
MailKeyValues.smtpMessageFrom(message).ifPresent(values::add);
55+
MailKeyValues.smtpMessageRecipients(message, RecipientType.TO).ifPresent(values::add);
56+
MailKeyValues.smtpMessageRecipients(message, RecipientType.CC).ifPresent(values::add);
57+
MailKeyValues.smtpMessageRecipients(message, RecipientType.BCC).ifPresent(values::add);
58+
59+
return KeyValues.of(values);
5360
}
5461

5562
}

Diff for: ‎micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/mail/InstrumentedTransport.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ public void sendMessage(Message msg, Address[] addresses) throws MessagingExcept
9292
try (Observation.Scope ignore = observation.openScope()) {
9393
// the Message-Id is set by the Transport (from the SMTP server) after sending
9494
this.delegate.sendMessage(msg, addresses);
95-
observation.highCardinalityKeyValue(MailKeyValues.smtpMessageId(msg));
95+
MailKeyValues.smtpMessageId(msg).ifPresent(observation::highCardinalityKeyValue);
9696
}
9797
catch (MessagingException error) {
9898
observation.error(error);

Diff for: ‎micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/mail/MailKeyValues.java

+33-37
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import java.util.Arrays;
2323
import java.util.Locale;
24+
import java.util.Optional;
2425
import java.util.stream.Collectors;
2526

2627
import io.micrometer.common.KeyValue;
@@ -36,39 +37,38 @@ class MailKeyValues {
3637
*/
3738
public static final String UNKNOWN = "unknown";
3839

39-
/**
40-
* The value is when the value is not set or empty.
41-
*/
42-
public static final String EMPTY = "";
43-
4440
private MailKeyValues() {
4541
}
4642

47-
static KeyValue smtpMessageFrom(Message message) {
48-
return safeExtractValue(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_FROM,
49-
() -> addressesToValue(message.getFrom()));
43+
static Optional<KeyValue> smtpMessageFrom(Message message) {
44+
return safeExtractValue(
45+
MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_FROM,
46+
() -> addressesToValue(message.getFrom()));
5047
}
5148

52-
static KeyValue smtpMessageRecipients(Message message, RecipientType recipientType) {
49+
static Optional<KeyValue> smtpMessageRecipients(Message message, RecipientType recipientType) {
5350
MailObservationDocumentation.HighCardinalityKeyNames key = MailObservationDocumentation.HighCardinalityKeyNames
5451
.valueOf("SMTP_MESSAGE_" + recipientType.toString().toUpperCase(Locale.ROOT));
5552
return safeExtractValue(key, () -> addressesToValue(message.getRecipients(recipientType)));
5653
}
5754

58-
static KeyValue smtpMessageSubject(Message message) {
55+
static Optional<KeyValue> smtpMessageSubject(Message message) {
5956

60-
return safeExtractValue(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_SUBJECT,
61-
message::getSubject);
57+
return safeExtractValue(
58+
MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_SUBJECT,
59+
() -> Optional.ofNullable(
60+
message.getSubject()));
6261
}
6362

64-
static KeyValue smtpMessageId(Message message) {
65-
return safeExtractValue(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_ID, () -> {
66-
String[] header = message.getHeader("Message-ID");
67-
if (header == null || header.length == 0) {
68-
return EMPTY;
69-
}
70-
return String.join(", ", header);
71-
});
63+
static Optional<KeyValue> smtpMessageId(Message message) {
64+
return safeExtractValue(
65+
MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_ID, () -> {
66+
String[] header = message.getHeader("Message-ID");
67+
if (header == null || header.length == 0) {
68+
return Optional.empty();
69+
}
70+
return Optional.of(String.join(", ", header));
71+
});
7272
}
7373

7474
static KeyValue serverAddress(MailSendObservationContext context) {
@@ -95,36 +95,32 @@ static KeyValue networkProtocolName(MailSendObservationContext context) {
9595
return KeyValue.of(LowCardinalityKeyNames.NETWORK_PROTOCOL_NAME, protocol);
9696
}
9797

98-
private static KeyValue safeExtractValue(KeyName key, ValueExtractor extractor) {
98+
private static Optional<KeyValue> safeExtractValue(KeyName key, ValueExtractor extractor) {
9999
String value;
100100
try {
101-
value = extractor.extract();
102-
if (value == null || value.isEmpty()) {
103-
value = EMPTY;
101+
Optional<String> extracted = extractor.extract();
102+
if (!extracted.isPresent()) {
103+
return Optional.empty();
104104
}
105-
}
106-
catch (MessagingException exc) {
105+
value = extracted.get();
106+
} catch (MessagingException exc) {
107107
value = UNKNOWN;
108108
}
109-
return key.withValue(value);
109+
return Optional.of(key.withValue(value));
110110
}
111111

112-
private static String addressesToValue(@Nullable Address[] addresses) {
113-
String value;
112+
private static Optional<String> addressesToValue(@Nullable Address[] addresses) {
114113
if (addresses == null || addresses.length == 0) {
115-
value = EMPTY;
114+
return Optional.empty();
116115
}
117-
else {
118-
value = Arrays.stream(addresses).map(Address::toString).collect(Collectors.joining(", "));
119-
}
120-
return value;
116+
String value = Arrays.stream(addresses).map(Address::toString)
117+
.collect(Collectors.joining(", "));
118+
return Optional.of(value);
121119
}
122120

123121
private interface ValueExtractor {
124122

125-
@Nullable
126-
String extract() throws MessagingException;
127-
123+
Optional<String> extract() throws MessagingException;
128124
}
129125

130126
}

Diff for: ‎micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/mail/DefaultMailSendObservationConventionTests.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ void shouldHaveHighCardinalityKeyValues() throws MessagingException {
5656
MailSendObservationContext context = new MailSendObservationContext(message, "smtp", "localhost", 25);
5757
assertThat(convention.getHighCardinalityKeyValues(context)).contains(
5858
KeyValue.of("smtp.message.from", "[email protected]"), KeyValue.of("smtp.message.to", "[email protected]"),
59-
KeyValue.of("smtp.message.cc", ""), KeyValue.of("smtp.message.bcc", ""),
6059
KeyValue.of("smtp.message.subject", "Test Subject"));
6160
}
6261

@@ -83,9 +82,8 @@ public Address[] getFrom() throws MessagingException {
8382
};
8483

8584
MailSendObservationContext context = new MailSendObservationContext(message, "smtp", "localhost", 25);
86-
assertThat(convention.getHighCardinalityKeyValues(context)).contains(
87-
KeyValue.of("smtp.message.from", "unknown"), KeyValue.of("smtp.message.to", ""),
88-
KeyValue.of("smtp.message.subject", ""));
85+
assertThat(convention.getHighCardinalityKeyValues(context))
86+
.contains(KeyValue.of("smtp.message.from", "unknown"));
8987
}
9088

9189
@Test

Diff for: ‎micrometer-jakarta9/src/test/java/io/micrometer/jakarta9/instrument/mail/MailKeyValuesTest.java

+31-22
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,9 @@
1717
package io.micrometer.jakarta9.instrument.mail;
1818

1919
import static io.micrometer.jakarta9.instrument.mail.MailKeyValues.UNKNOWN;
20-
import static io.micrometer.jakarta9.instrument.mail.MailKeyValues.EMPTY;
20+
import static io.micrometer.jakarta9.instrument.mail.MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_FROM;
21+
import static io.micrometer.jakarta9.instrument.mail.MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_SUBJECT;
22+
import static io.micrometer.jakarta9.instrument.mail.MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_ID;
2123
import static org.assertj.core.api.Assertions.assertThat;
2224

2325
import jakarta.mail.Message;
@@ -45,15 +47,15 @@ void smtpMessageFrom_WhenFromFails_ReturnsUnknown() throws MessagingException {
4547
var message = Mockito.mock(Message.class);
4648
Mockito.when(message.getFrom()).thenThrow(new MessagingException("test exception"));
4749
var result = MailKeyValues.smtpMessageFrom(message);
48-
assertThat(result.getValue()).isEqualTo(UNKNOWN);
50+
assertThat(result).isPresent().hasValue(SMTP_MESSAGE_FROM.withValue(UNKNOWN));
4951
}
5052

5153
@Test
5254
@DisplayName("Should return EMPTY when 'from' address is unset")
5355
void smtpMessageFrom_WhenFromUnset_ReturnsNone() {
5456
var message = new MimeMessage((Session) null);
5557
var result = MailKeyValues.smtpMessageFrom(message);
56-
assertThat(result.getValue()).isEqualTo(EMPTY);
58+
assertThat(result).isEmpty();
5759
}
5860

5961
@Test
@@ -62,7 +64,7 @@ void smtpMessageFrom_WhenFromEmpty_ReturnsNone() throws MessagingException {
6264
var message = new MimeMessage((Session) null);
6365
message.addFrom(new InternetAddress[0]);
6466
var result = MailKeyValues.smtpMessageFrom(message);
65-
assertThat(result.getValue()).isEqualTo(EMPTY);
67+
assertThat(result).isEmpty();
6668
}
6769

6870
@Test
@@ -71,7 +73,7 @@ void smtpMessageFrom_WhenOneFromAddress_ReturnsAddress() throws MessagingExcepti
7173
var message = new MimeMessage((Session) null);
7274
message.addFrom(new InternetAddress[] { new InternetAddress("[email protected]") });
7375
var result = MailKeyValues.smtpMessageFrom(message);
74-
assertThat(result.getValue()).isEqualTo("[email protected]");
76+
assertThat(result).isPresent().hasValue(SMTP_MESSAGE_FROM.withValue("[email protected]"));
7577
}
7678

7779
@Test
@@ -81,7 +83,7 @@ void smtpMessageFrom_WhenMultipleFromAddresses_ReturnsAddresses() throws Messagi
8183
message.addFrom(new InternetAddress[] { new InternetAddress("[email protected]"),
8284
new InternetAddress("[email protected]") });
8385
var result = MailKeyValues.smtpMessageFrom(message);
84-
assertThat(result.getValue()).isEqualTo("[email protected], [email protected]");
86+
assertThat(result).isPresent().hasValue(SMTP_MESSAGE_FROM.withValue("[email protected], [email protected]"));
8587
}
8688

8789
@ParameterizedTest
@@ -90,16 +92,20 @@ void smtpMessageFrom_WhenMultipleFromAddresses_ReturnsAddresses() throws Messagi
9092
void smtpMessageRecipients_WhenRecipientsUnset_ReturnsNone(RecipientType recipientType) {
9193
var message = new MimeMessage((Session) null);
9294
var result = MailKeyValues.smtpMessageRecipients(message, recipientType);
93-
assertThat(result.getValue()).isEqualTo(EMPTY);
95+
assertThat(result).isEmpty();
9496
}
9597

96-
@Test
98+
@ParameterizedTest
99+
@MethodSource("recipientTypes")
97100
@DisplayName("Should return UNKNOWN when recipient retrieval fails")
98-
void smtpMessageRecipients_WhenRecipientsFail_ReturnsUnknown() throws MessagingException {
101+
void smtpMessageRecipients_WhenRecipientsFail_ReturnsUnknown(RecipientType recipientType)
102+
throws MessagingException {
103+
MailObservationDocumentation.HighCardinalityKeyNames key = MailObservationDocumentation.HighCardinalityKeyNames
104+
.valueOf("SMTP_MESSAGE_" + recipientType.toString().toUpperCase(Locale.ROOT));
99105
var message = Mockito.mock(Message.class);
100106
Mockito.when(message.getRecipients(ArgumentMatchers.any())).thenThrow(new MessagingException("test exception"));
101-
var result = MailKeyValues.smtpMessageRecipients(message, RecipientType.TO);
102-
assertThat(result.getValue()).isEqualTo(UNKNOWN);
107+
var result = MailKeyValues.smtpMessageRecipients(message, recipientType);
108+
assertThat(result).isPresent().hasValue(key.withValue(UNKNOWN));
103109
}
104110

105111
@ParameterizedTest
@@ -109,33 +115,35 @@ void smtpMessageRecipients_WhenRecipientsEmpty_ReturnsNone(RecipientType recipie
109115
var message = new MimeMessage((Session) null);
110116
message.addRecipients(recipientType, new InternetAddress[0]);
111117
var result = MailKeyValues.smtpMessageRecipients(message, recipientType);
112-
assertThat(result.getValue()).isEqualTo(EMPTY);
118+
assertThat(result).isEmpty();
113119
}
114120

115121
@ParameterizedTest
116122
@MethodSource("recipientTypes")
117123
@DisplayName("Should return single recipient when one is set")
118124
void smtpMessageRecipients_WhenOneRecipient_ReturnsRecipient(RecipientType recipientType)
119125
throws MessagingException {
126+
MailObservationDocumentation.HighCardinalityKeyNames key = MailObservationDocumentation.HighCardinalityKeyNames
127+
.valueOf("SMTP_MESSAGE_" + recipientType.toString().toUpperCase(Locale.ROOT));
120128
var message = new MimeMessage((Session) null);
121129
message.addRecipients(recipientType, new InternetAddress[] { new InternetAddress("[email protected]") });
122130
var result = MailKeyValues.smtpMessageRecipients(message, recipientType);
123-
assertThat(result.getValue()).isEqualTo("[email protected]");
131+
assertThat(result).isPresent().hasValue(key.withValue("[email protected]"));
124132
}
125133

126134
@ParameterizedTest
127135
@MethodSource("recipientTypes")
128136
@DisplayName("Should return multiple recipients when more than one is set")
129137
void smtpMessageRecipients_WhenMultipleRecipients_ReturnsRecipients(RecipientType recipientType)
130138
throws MessagingException {
139+
MailObservationDocumentation.HighCardinalityKeyNames key = MailObservationDocumentation.HighCardinalityKeyNames
140+
.valueOf("SMTP_MESSAGE_" + recipientType.toString().toUpperCase(Locale.ROOT));
131141
var message = new MimeMessage((Session) null);
132142
message.addRecipients(recipientType, new InternetAddress[] { new InternetAddress("[email protected]"),
133143
new InternetAddress("[email protected]") });
134144
var result = MailKeyValues.smtpMessageRecipients(message, recipientType);
135145

136-
var expectedKeyName = "smtp.message." + recipientType.toString().toLowerCase(Locale.ROOT);
137-
assertThat(result.getKey()).isEqualTo(expectedKeyName);
138-
assertThat(result.getValue()).isEqualTo("[email protected], [email protected]");
146+
assertThat(result).isPresent().hasValue(key.withValue("[email protected], [email protected]"));
139147
}
140148

141149
public static Stream<Arguments> recipientTypes() {
@@ -149,7 +157,7 @@ void smtpMessageSubject_WhenSubjectSet_ReturnsSubject() throws MessagingExceptio
149157
var message = new MimeMessage((Session) null);
150158
message.setSubject("test subject");
151159
var result = MailKeyValues.smtpMessageSubject(message);
152-
assertThat(result.getValue()).isEqualTo("test subject");
160+
assertThat(result).isPresent().hasValue(SMTP_MESSAGE_SUBJECT.withValue("test subject"));
153161
}
154162

155163
@Test
@@ -158,15 +166,16 @@ void smtpMessageSubject_WhenSubjectFails_ReturnsUnknown() throws MessagingExcept
158166
var message = Mockito.mock(Message.class);
159167
Mockito.when(message.getSubject()).thenThrow(new MessagingException("test exception"));
160168
var result = MailKeyValues.smtpMessageSubject(message);
161-
assertThat(result.getValue()).isEqualTo(UNKNOWN);
169+
assertThat(result).isPresent().hasValue(SMTP_MESSAGE_SUBJECT.withValue(UNKNOWN));
162170
}
163171

164172
@Test
165173
@DisplayName("Should return EMPTY when subject is unset")
166174
void smtpMessageSubject_WhenSubjectUnset_ReturnsNone() {
167175
var message = new MimeMessage((Session) null);
168176
var result = MailKeyValues.smtpMessageSubject(message);
169-
assertThat(result.getValue()).isEqualTo(EMPTY);
177+
assertThat(result).isEmpty();
178+
170179
}
171180

172181
@Test
@@ -215,15 +224,15 @@ void smtpMessageId_WhenHeaderFails_ReturnsUnknown() throws MessagingException {
215224
var message = Mockito.mock(Message.class);
216225
Mockito.when(message.getHeader("Message-ID")).thenThrow(new MessagingException("test exception"));
217226
var result = MailKeyValues.smtpMessageId(message);
218-
assertThat(result.getValue()).isEqualTo(UNKNOWN);
227+
assertThat(result).isPresent().hasValue(SMTP_MESSAGE_ID.withValue(UNKNOWN));
219228
}
220229

221230
@Test
222231
@DisplayName("Should return EMPTY when Message-ID header is missing")
223232
void smtpMessageId_WhenHeaderMissing_ReturnsUnknown() {
224233
var message = new MimeMessage((Session) null);
225234
var result = MailKeyValues.smtpMessageId(message);
226-
assertThat(result.getValue()).isEqualTo(EMPTY);
235+
assertThat(result).isEmpty();
227236
}
228237

229238
@Test
@@ -232,7 +241,7 @@ void smtpMessageId_WhenHeaderSet_ReturnsMessageId() throws MessagingException {
232241
var message = new MimeMessage((Session) null);
233242
message.addHeader("Message-ID", "[email protected]");
234243
var result = MailKeyValues.smtpMessageId(message);
235-
assertThat(result.getValue()).isEqualTo("[email protected]");
244+
assertThat(result).isPresent().hasValue(SMTP_MESSAGE_ID.withValue("[email protected]"));
236245
}
237246

238247
@Test

0 commit comments

Comments
 (0)