Skip to content

Commit 02f7ab3

Browse files
famaridonfamaridon
committed
Improve code based on comments
Signed-off-by: famaridon <[email protected]>
1 parent 909dbe2 commit 02f7ab3

File tree

7 files changed

+378
-69
lines changed

7 files changed

+378
-69
lines changed

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import io.micrometer.common.KeyValues;
1919
import jakarta.mail.Message;
20+
import jakarta.mail.Message.RecipientType;
2021

2122
/**
2223
* Default implementation for {@link MailSendObservationConvention}.
@@ -44,7 +45,10 @@ public KeyValues getLowCardinalityKeyValues(MailSendObservationContext context)
4445
@Override
4546
public KeyValues getHighCardinalityKeyValues(MailSendObservationContext context) {
4647
Message message = context.getCarrier();
47-
return KeyValues.of(MailKeyValues.smtpMessageFrom(message), MailKeyValues.smtpMessageTo(message),
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),
4852
MailKeyValues.smtpMessageSubject(message));
4953
}
5054

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public void sendMessage(Message msg, Address[] addresses) throws MessagingExcept
9090

9191
observation.start();
9292
try (Observation.Scope ignore = observation.openScope()) {
93-
// the Message-Id is set by the SMTP after sending the message
93+
// the Message-Id is set by the Transport (from the SMTP server) after sending
9494
this.delegate.sendMessage(msg, addresses);
9595
observation.highCardinalityKeyValue(MailKeyValues.smtpMessageId(msg));
9696
}

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

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

18+
import io.micrometer.common.docs.KeyName;
19+
import io.micrometer.common.lang.Nullable;
1820
import io.micrometer.jakarta9.instrument.mail.MailObservationDocumentation.LowCardinalityKeyNames;
1921

2022
import java.util.Arrays;
23+
import java.util.Locale;
2124
import java.util.stream.Collectors;
2225

2326
import io.micrometer.common.KeyValue;
@@ -28,99 +31,100 @@
2831

2932
class MailKeyValues {
3033

31-
private static final KeyValue SMTP_MESSAGE_FROM_UNKNOWN = KeyValue
32-
.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_FROM, "unknown");
34+
/**
35+
* The value is when value can't be determined.
36+
*/
37+
public static final String UNKNOWN = "unknown";
3338

34-
private static final KeyValue SMTP_MESSAGE_TO_UNKNOWN = KeyValue
35-
.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_TO, "unknown");
39+
/**
40+
* The value is when the value is not set or empty.
41+
*/
42+
public static final String EMPTY = "";
3643

37-
private static final KeyValue SMTP_MESSAGE_SUBJECT_UNKNOWN = KeyValue
38-
.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_SUBJECT, "unknown");
44+
private MailKeyValues() {
45+
}
3946

40-
private static final KeyValue SMTP_MESSAGE_ID_UNKNOWN = KeyValue
41-
.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_ID, "unknown");
47+
static KeyValue smtpMessageFrom(Message message) {
48+
return safeExtractValue(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_FROM,
49+
() -> addressesToValue(message.getFrom()));
50+
}
4251

43-
private static final KeyValue SERVER_ADDRESS_UNKNOWN = KeyValue.of(LowCardinalityKeyNames.SERVER_ADDRESS,
44-
"unknown");
52+
static KeyValue smtpMessageRecipients(Message message, RecipientType recipientType) {
53+
MailObservationDocumentation.HighCardinalityKeyNames key = MailObservationDocumentation.HighCardinalityKeyNames
54+
.valueOf("SMTP_MESSAGE_" + recipientType.toString().toUpperCase(Locale.ROOT));
55+
return safeExtractValue(key, () -> addressesToValue(message.getRecipients(recipientType)));
56+
}
4557

46-
private static final KeyValue SERVER_PORT_UNKNOWN = KeyValue.of(LowCardinalityKeyNames.SERVER_PORT, "unknown");
58+
static KeyValue smtpMessageSubject(Message message) {
4759

48-
private MailKeyValues() {
60+
return safeExtractValue(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_SUBJECT,
61+
message::getSubject);
4962
}
5063

51-
static KeyValue smtpMessageFrom(Message message) {
52-
try {
53-
if (message.getFrom() == null || message.getFrom().length == 0) {
54-
return SMTP_MESSAGE_FROM_UNKNOWN;
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;
5569
}
56-
String fromString = Arrays.stream(message.getFrom())
57-
.map(Address::toString)
58-
.collect(Collectors.joining(", "));
59-
return KeyValue.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_FROM, fromString);
60-
}
61-
catch (MessagingException exc) {
62-
return SMTP_MESSAGE_FROM_UNKNOWN;
63-
}
70+
return String.join(", ", header);
71+
});
6472
}
6573

66-
static KeyValue smtpMessageTo(Message message) {
67-
try {
68-
Address[] recipients = message.getRecipients(RecipientType.TO);
69-
if (recipients == null || recipients.length == 0) {
70-
return SMTP_MESSAGE_TO_UNKNOWN;
71-
}
72-
String recipientsString = Arrays.stream(recipients)
73-
.map(Address::toString)
74-
.collect(Collectors.joining(", "));
75-
return KeyValue.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_TO, recipientsString);
76-
}
77-
catch (MessagingException exc) {
78-
return SMTP_MESSAGE_TO_UNKNOWN;
74+
static KeyValue serverAddress(MailSendObservationContext context) {
75+
String host = context.getHost();
76+
if (host == null || host.isEmpty()) {
77+
host = UNKNOWN;
7978
}
79+
return LowCardinalityKeyNames.SERVER_ADDRESS.withValue(host);
8080
}
8181

82-
static KeyValue smtpMessageSubject(Message message) {
83-
try {
84-
if (message.getSubject() == null) {
85-
return SMTP_MESSAGE_SUBJECT_UNKNOWN;
86-
}
87-
return KeyValue.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_SUBJECT,
88-
message.getSubject());
82+
static KeyValue serverPort(MailSendObservationContext context) {
83+
String port = UNKNOWN;
84+
if (context.getPort() > 0) {
85+
port = String.valueOf(context.getPort());
8986
}
90-
catch (MessagingException exc) {
91-
return SMTP_MESSAGE_SUBJECT_UNKNOWN;
87+
return LowCardinalityKeyNames.SERVER_PORT.withValue(port);
88+
}
89+
90+
static KeyValue networkProtocolName(MailSendObservationContext context) {
91+
String protocol = context.getProtocol();
92+
if (protocol == null || protocol.isEmpty()) {
93+
protocol = UNKNOWN;
9294
}
95+
return KeyValue.of(LowCardinalityKeyNames.NETWORK_PROTOCOL_NAME, protocol);
9396
}
9497

95-
static KeyValue smtpMessageId(Message message) {
98+
private static KeyValue safeExtractValue(KeyName key, ValueExtractor extractor) {
99+
String value;
96100
try {
97-
if (message.getHeader("Message-ID") == null) {
98-
return SMTP_MESSAGE_ID_UNKNOWN;
101+
value = extractor.extract();
102+
if (value == null || value.isEmpty()) {
103+
value = EMPTY;
99104
}
100-
return KeyValue.of(MailObservationDocumentation.HighCardinalityKeyNames.SMTP_MESSAGE_ID,
101-
message.getHeader("Message-ID")[0]);
102105
}
103106
catch (MessagingException exc) {
104-
return SMTP_MESSAGE_ID_UNKNOWN;
107+
value = UNKNOWN;
105108
}
109+
return key.withValue(value);
106110
}
107111

108-
static KeyValue serverAddress(MailSendObservationContext context) {
109-
if (context.getHost() == null) {
110-
return SERVER_ADDRESS_UNKNOWN;
112+
private static String addressesToValue(@Nullable Address[] addresses) {
113+
String value;
114+
if (addresses == null || addresses.length == 0) {
115+
value = EMPTY;
111116
}
112-
return KeyValue.of(LowCardinalityKeyNames.SERVER_ADDRESS, context.getHost());
113-
}
114-
115-
static KeyValue serverPort(MailSendObservationContext context) {
116-
if (context.getPort() <= 0) {
117-
return SERVER_PORT_UNKNOWN;
117+
else {
118+
value = Arrays.stream(addresses).map(Address::toString).collect(Collectors.joining(", "));
118119
}
119-
return KeyValue.of(LowCardinalityKeyNames.SERVER_PORT, String.valueOf(context.getPort()));
120+
return value;
120121
}
121122

122-
static KeyValue networkProtocolName(MailSendObservationContext context) {
123-
return KeyValue.of(LowCardinalityKeyNames.NETWORK_PROTOCOL_NAME, context.getProtocol());
123+
private interface ValueExtractor {
124+
125+
@Nullable
126+
String extract() throws MessagingException;
127+
124128
}
125129

126130
}

‎micrometer-jakarta9/src/main/java/io/micrometer/jakarta9/instrument/mail/MailObservationDocumentation.java

+39
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,27 @@ public KeyName[] getHighCardinalityKeyNames() {
5151

5252
public enum LowCardinalityKeyNames implements KeyName {
5353

54+
/**
55+
* (SMTP) Server address used for sending mails.
56+
*/
5457
SERVER_ADDRESS {
5558
@Override
5659
public String asString() {
5760
return "server.address";
5861
}
5962
},
63+
/**
64+
* (SMTP) Server port used for sending mails.
65+
*/
6066
SERVER_PORT {
6167
@Override
6268
public String asString() {
6369
return "server.port";
6470
}
6571
},
72+
/**
73+
* Network protocol used for sending mails.
74+
*/
6675
NETWORK_PROTOCOL_NAME {
6776
@Override
6877
public String asString() {
@@ -74,24 +83,54 @@ public String asString() {
7483

7584
public enum HighCardinalityKeyNames implements KeyName {
7685

86+
/**
87+
* Sender of the mail.
88+
*/
7789
SMTP_MESSAGE_FROM {
7890
@Override
7991
public String asString() {
8092
return "smtp.message.from";
8193
}
8294
},
95+
/**
96+
* Primary (TO) recipient(s) of the mail.
97+
*/
8398
SMTP_MESSAGE_TO {
8499
@Override
85100
public String asString() {
86101
return "smtp.message.to";
87102
}
88103
},
104+
/**
105+
* Carbon copy (CC) recipient(s) of the mail.
106+
*/
107+
SMTP_MESSAGE_CC {
108+
@Override
109+
public String asString() {
110+
return "smtp.message.cc";
111+
}
112+
},
113+
/**
114+
* Blind carbon copy (BCC) recipient(s) of the mail.
115+
*/
116+
SMTP_MESSAGE_BCC {
117+
@Override
118+
public String asString() {
119+
return "smtp.message.bcc";
120+
}
121+
},
122+
/**
123+
* Subject line of the mail.
124+
*/
89125
SMTP_MESSAGE_SUBJECT {
90126
@Override
91127
public String asString() {
92128
return "smtp.message.subject";
93129
}
94130
},
131+
/**
132+
* Message ID received from the SMTP server.
133+
*/
95134
SMTP_MESSAGE_ID {
96135
@Override
97136
public String asString() {

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

+7-3
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727

2828
import static org.assertj.core.api.Assertions.assertThat;
2929

30+
/**
31+
* Tests for {@link DefaultMailSendObservationConvention}.
32+
*/
3033
class DefaultMailSendObservationConventionTests {
3134

3235
private final DefaultMailSendObservationConvention convention = new DefaultMailSendObservationConvention();
@@ -53,6 +56,7 @@ void shouldHaveHighCardinalityKeyValues() throws MessagingException {
5356
MailSendObservationContext context = new MailSendObservationContext(message, "smtp", "localhost", 25);
5457
assertThat(convention.getHighCardinalityKeyValues(context)).contains(
5558
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", ""),
5660
KeyValue.of("smtp.message.subject", "Test Subject"));
5761
}
5862

@@ -70,7 +74,7 @@ void shouldHaveLowCardinalityKeyValues() throws MessagingException {
7074
}
7175

7276
@Test
73-
void shouldHandleMessagingException() throws MessagingException {
77+
void shouldHandleMessagingException() {
7478
MimeMessage message = new MimeMessage((Session) null) {
7579
@Override
7680
public Address[] getFrom() throws MessagingException {
@@ -80,8 +84,8 @@ public Address[] getFrom() throws MessagingException {
8084

8185
MailSendObservationContext context = new MailSendObservationContext(message, "smtp", "localhost", 25);
8286
assertThat(convention.getHighCardinalityKeyValues(context)).contains(
83-
KeyValue.of("smtp.message.from", "unknown"), KeyValue.of("smtp.message.to", "unknown"),
84-
KeyValue.of("smtp.message.subject", "unknown"));
87+
KeyValue.of("smtp.message.from", "unknown"), KeyValue.of("smtp.message.to", ""),
88+
KeyValue.of("smtp.message.subject", ""));
8589
}
8690

8791
@Test

0 commit comments

Comments
 (0)