Skip to content

Commit a48b5c0

Browse files
philsttrphilsttr
committed
Address review comments
Use cached AttributeKeys for dynamic key names in log mappers. Rename setAttributeMaybeEventName to setAttributeOrEventName Clarify capture event name description in readme
1 parent 54f2c02 commit a48b5c0

File tree

10 files changed

+87
-63
lines changed

10 files changed

+87
-63
lines changed

‎instrumentation/jboss-logmanager/README.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# Settings for the JBoss Log Manager instrumentation
22

3-
| System property | Type | Default | Description |
4-
|-----------------------------------------------------------------------------|---------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
5-
| `otel.instrumentation.jboss-logmanager.experimental-log-attributes` | Boolean | `false` | Enable the capture of experimental log attributes `thread.name` and `thread.id`. |
6-
| `otel.instrumentation.jboss-logmanager.experimental.capture-mdc-attributes` | String | | Comma separated list of MDC attributes to capture. Use the wildcard character `*` to capture all attributes. |
7-
| `otel.instrumentation.jboss-logmanager.experimental.capture-event-name` | Boolean | `false` | Enable the capture of the log event name from the `event.name` attribute (captured via one of the above means). When true, the `event.name` attribute will be used as the log event name, and the `event.name` attribute will be removed. |
3+
| System property | Type | Default | Description |
4+
|-----------------------------------------------------------------------------|---------|---------|------------------------------------------------------------------------------------------------------------------------------------|
5+
| `otel.instrumentation.jboss-logmanager.experimental-log-attributes` | Boolean | `false` | Enable the capture of experimental log attributes `thread.name` and `thread.id`. |
6+
| `otel.instrumentation.jboss-logmanager.experimental.capture-mdc-attributes` | String | | Comma separated list of MDC attributes to capture. Use the wildcard character `*` to capture all attributes. |
7+
| `otel.instrumentation.jboss-logmanager.experimental.capture-event-name` | Boolean | `false` | Enable moving the `event.name` attribute (captured by one of the other mechanisms of capturing attributes) to the log event name. |

‎instrumentation/jboss-logmanager/jboss-logmanager-appender-1.1/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jbosslogmanager/appender/v1_1/LoggingEventMapper.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@
99
import static java.util.concurrent.TimeUnit.MILLISECONDS;
1010

1111
import io.opentelemetry.api.GlobalOpenTelemetry;
12+
import io.opentelemetry.api.common.AttributeKey;
1213
import io.opentelemetry.api.incubator.logs.ExtendedLogRecordBuilder;
1314
import io.opentelemetry.api.logs.LogRecordBuilder;
1415
import io.opentelemetry.api.logs.Severity;
1516
import io.opentelemetry.context.Context;
17+
import io.opentelemetry.instrumentation.api.internal.cache.Cache;
1618
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
1719
import io.opentelemetry.semconv.incubating.ThreadIncubatingAttributes;
1820
import java.util.List;
@@ -26,8 +28,10 @@ public final class LoggingEventMapper {
2628

2729
public static final LoggingEventMapper INSTANCE = new LoggingEventMapper();
2830

31+
private static final Cache<String, AttributeKey<String>> mdcAttributeKeys = Cache.bounded(100);
32+
2933
// copied from EventIncubatingAttributes
30-
private static final String EVENT_NAME = "event.name";
34+
private static final AttributeKey<String> EVENT_NAME = AttributeKey.stringKey("event.name");
3135

3236
private final List<String> captureMdcAttributes;
3337

@@ -103,24 +107,29 @@ private void captureMdcAttributes(LogRecordBuilder builder) {
103107
if (captureAllMdcAttributes) {
104108
if (context != null) {
105109
for (Map.Entry<String, String> entry : context.entrySet()) {
106-
setAttributeMaybeEventName(builder, entry.getKey(), String.valueOf(entry.getValue()));
110+
setAttributeOrEventName(builder, getMdcAttributeKey(entry.getKey()), entry.getValue());
107111
}
108112
}
109113
return;
110114
}
111115

112116
for (String key : captureMdcAttributes) {
113117
Object value = context.get(key);
114-
setAttributeMaybeEventName(builder, key, value.toString());
118+
setAttributeOrEventName(builder, getMdcAttributeKey(key), value);
115119
}
116120
}
117121

118-
private void setAttributeMaybeEventName(LogRecordBuilder builder, String key, String value) {
122+
public static AttributeKey<String> getMdcAttributeKey(String key) {
123+
return mdcAttributeKeys.computeIfAbsent(key, AttributeKey::stringKey);
124+
}
125+
126+
private void setAttributeOrEventName(
127+
LogRecordBuilder builder, AttributeKey<String> key, Object value) {
119128
if (value != null) {
120129
if (captureEventName && key.equals(EVENT_NAME)) {
121-
builder.setEventName(value);
130+
builder.setEventName(value.toString());
122131
} else {
123-
builder.setAttribute(key, value);
132+
builder.setAttribute(key, value.toString());
124133
}
125134
}
126135
}

‎instrumentation/log4j/log4j-appender-2.17/javaagent/README.md‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
# Settings for the Log4j Appender instrumentation
22

3-
| System property | Type | Default | Description |
4-
|-----------------------------------------------------------------------------------|---------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
5-
| `otel.instrumentation.log4j-appender.experimental-log-attributes` | Boolean | `false` | Enable the capture of experimental log attributes `thread.name` and `thread.id`. |
6-
| `otel.instrumentation.log4j-appender.experimental.capture-code-attributes` | Boolean | `false` | Enable the capture of [source code attributes]. Note that capturing source code attributes at logging sites might add a performance overhead. |
7-
| `otel.instrumentation.log4j-appender.experimental.capture-map-message-attributes` | Boolean | `false` | Enable the capture of `MapMessage` attributes. |
8-
| `otel.instrumentation.log4j-appender.experimental.capture-marker-attribute` | Boolean | `false` | Enable the capture of Log4j markers as attributes. |
9-
| `otel.instrumentation.log4j-appender.experimental.capture-mdc-attributes` | String | | Comma separated list of context data attributes to capture. Use the wildcard character `*` to capture all attributes. |
10-
| `otel.instrumentation.log4j-appender.experimental.capture-event-name` | Boolean | `false` | Enable the capture of the log event name from the `event.name` attribute (captured via one of the above means). When true, the `event.name` attribute will be used as the log event name, and the `event.name` attribute will be removed. |
3+
| System property | Type | Default | Description |
4+
|-----------------------------------------------------------------------------------|---------|---------|-----------------------------------------------------------------------------------------------------------------------------------------------|
5+
| `otel.instrumentation.log4j-appender.experimental-log-attributes` | Boolean | `false` | Enable the capture of experimental log attributes `thread.name` and `thread.id`. |
6+
| `otel.instrumentation.log4j-appender.experimental.capture-code-attributes` | Boolean | `false` | Enable the capture of [source code attributes]. Note that capturing source code attributes at logging sites might add a performance overhead. |
7+
| `otel.instrumentation.log4j-appender.experimental.capture-map-message-attributes` | Boolean | `false` | Enable the capture of `MapMessage` attributes. |
8+
| `otel.instrumentation.log4j-appender.experimental.capture-marker-attribute` | Boolean | `false` | Enable the capture of Log4j markers as attributes. |
9+
| `otel.instrumentation.log4j-appender.experimental.capture-mdc-attributes` | String | | Comma separated list of context data attributes to capture. Use the wildcard character `*` to capture all attributes. |
10+
| `otel.instrumentation.log4j-appender.experimental.capture-event-name` | Boolean | `false` | Enable moving the `event.name` attribute (captured by one of the other mechanisms of capturing attributes) to the log event name. |
1111

1212
[source code attributes]: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attributes.md#source-code-attributes

‎instrumentation/log4j/log4j-appender-2.17/library/README.md‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,14 @@ Setting can be configured as XML attributes, for example:
9292

9393
The available settings are:
9494

95-
| XML Attribute | Type | Default | Description |
96-
|------------------------------------|---------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
97-
| `captureExperimentalAttributes` | Boolean | `false` | Enable the capture of experimental log attributes `thread.name` and `thread.id`. |
98-
| `captureCodeAttributes` | Boolean | `false` | Enable the capture of [source code attributes]. Note that capturing source code attributes at logging sites might add a performance overhead. |
99-
| `captureMapMessageAttributes` | Boolean | `false` | Enable the capture of `MapMessage` attributes. |
100-
| `captureMarkerAttribute` | Boolean | `false` | Enable the capture of Log4j markers as attributes. |
101-
| `captureContextDataAttributes` | String | | Comma separated list of context data attributes to capture. Use the wildcard character `*` to capture all attributes. |
102-
| `captureEventName` | Boolean | `false` | Enable the capture of the log event name from the `event.name` attribute (captured via one of the above means). When true, the `event.name` attribute will be used as the log event name, and the `event.name` attribute will be removed. |
103-
| `numLogsCapturedBeforeOtelInstall` | Integer | 1000 | Log telemetry is emitted after the initialization of the OpenTelemetry Log4j appender with an OpenTelemetry object. This setting allows you to modify the size of the cache used to replay the first logs. |
95+
| XML Attribute | Type | Default | Description |
96+
|------------------------------------|---------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
97+
| `captureExperimentalAttributes` | Boolean | `false` | Enable the capture of experimental log attributes `thread.name` and `thread.id`. |
98+
| `captureCodeAttributes` | Boolean | `false` | Enable the capture of [source code attributes]. Note that capturing source code attributes at logging sites might add a performance overhead. |
99+
| `captureMapMessageAttributes` | Boolean | `false` | Enable the capture of `MapMessage` attributes. |
100+
| `captureMarkerAttribute` | Boolean | `false` | Enable the capture of Log4j markers as attributes. |
101+
| `captureContextDataAttributes` | String | | Comma separated list of context data attributes to capture. Use the wildcard character `*` to capture all attributes. |
102+
| `captureEventName` | Boolean | `false` | Enable moving the `event.name` attribute (captured by one of the other mechanisms of capturing attributes) to the log event name. |
103+
| `numLogsCapturedBeforeOtelInstall` | Integer | 1000 | Log telemetry is emitted after the initialization of the OpenTelemetry Log4j appender with an OpenTelemetry object. This setting allows you to modify the size of the cache used to replay the first logs. |
104104

105105
[source code attributes]: https://github.com/open-telemetry/semantic-conventions/blob/main/docs/general/attributes.md#source-code-attributes

‎instrumentation/log4j/log4j-appender-2.17/library/src/main/java/io/opentelemetry/instrumentation/log4j/appender/v2_17/internal/LogEventMapper.java‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,12 @@ public final class LogEventMapper<T> {
4141
private static final AttributeKey<Long> THREAD_ID = AttributeKey.longKey("thread.id");
4242
private static final AttributeKey<String> THREAD_NAME = AttributeKey.stringKey("thread.name");
4343
// copied from EventIncubatingAttributes
44-
private static final String EVENT_NAME = "event.name";
44+
private static final AttributeKey<String> EVENT_NAME = AttributeKey.stringKey("event.name");
4545

4646
private static final String SPECIAL_MAP_MESSAGE_ATTRIBUTE = "message";
4747

48+
private static final Cache<String, AttributeKey<String>> contextDataAttributeKeyCache =
49+
Cache.bounded(100);
4850
private static final Cache<String, AttributeKey<String>> mapMessageAttributeKeyCache =
4951
Cache.bounded(100);
5052

@@ -206,26 +208,32 @@ void captureContextDataAttributes(LogRecordBuilder builder, T contextData) {
206208

207209
if (captureAllContextDataAttributes) {
208210
contextDataAccessor.forEach(
209-
contextData, (key, value) -> setAttributeMaybeEventName(builder, key, value));
211+
contextData,
212+
(key, value) -> setAttributeOrEventName(builder, getContextDataAttributeKey(key), value));
210213
return;
211214
}
212215

213216
for (String key : captureContextDataAttributes) {
214217
String value = contextDataAccessor.getValue(contextData, key);
215-
setAttributeMaybeEventName(builder, key, value);
218+
setAttributeOrEventName(builder, getContextDataAttributeKey(key), value);
216219
}
217220
}
218221

219-
private void setAttributeMaybeEventName(LogRecordBuilder builder, String key, String value) {
222+
private void setAttributeOrEventName(
223+
LogRecordBuilder builder, AttributeKey<String> key, Object value) {
220224
if (value != null) {
221225
if (captureEventName && key.equals(EVENT_NAME)) {
222-
builder.setEventName(value);
226+
builder.setEventName(value.toString());
223227
} else {
224-
builder.setAttribute(key, value);
228+
builder.setAttribute(key, value.toString());
225229
}
226230
}
227231
}
228232

233+
public static AttributeKey<String> getContextDataAttributeKey(String key) {
234+
return contextDataAttributeKeyCache.computeIfAbsent(key, AttributeKey::stringKey);
235+
}
236+
229237
public static AttributeKey<String> getMapMessageAttributeKey(String key) {
230238
return mapMessageAttributeKeyCache.computeIfAbsent(
231239
key, k -> AttributeKey.stringKey("log4j.map_message." + k));

‎instrumentation/log4j/log4j-appender-2.17/library/src/test/java/io/opentelemetry/instrumentation/log4j/appender/v2_17/internal/LogEventMapperTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ void testSome() {
6565
mapper.captureContextDataAttributes(builder, contextData);
6666

6767
// then
68-
verify(builder).setAttribute("key2", "value2");
68+
verify(builder).setAttribute(AttributeKey.stringKey("key2"), "value2");
6969
verifyNoMoreInteractions(builder);
7070
}
7171

@@ -90,8 +90,8 @@ void testAll() {
9090
mapper.captureContextDataAttributes(builder, contextData);
9191

9292
// then
93-
verify(builder).setAttribute("key1", "value1");
94-
verify(builder).setAttribute("key2", "value2");
93+
verify(builder).setAttribute(AttributeKey.stringKey("key1"), "value1");
94+
verify(builder).setAttribute(AttributeKey.stringKey("key2"), "value2");
9595
verifyNoMoreInteractions(builder);
9696
}
9797

0 commit comments

Comments
 (0)