Implement otel.event.name#16220
Conversation
| setAttributeOrEventName(builder, getMdcAttributeKey(key), value); | ||
| // Always capture otel.event.name as event name, taking precedence over event.name | ||
| if (context != null) { | ||
| String otelEventName = context.get("otel.event.name"); |
There was a problem hiding this comment.
MDC taking priority over explicit evenName could easily result in setting names on arbitrary logs under the context.
Would it make sense to avoid using otel.event.name from MDC at all (given it's not targeted)?
e53ed84 to
12fda96
Compare
2135bd6 to
87af7b3
Compare
| | `otel.instrumentation.logback-appender.experimental.capture-mdc-attributes` | String | | Comma separated list of MDC attributes to capture. Use the wildcard character `*` to capture all attributes. | | ||
| | `otel.instrumentation.logback-appender.experimental.capture-event-name` | Boolean | `false` | **Deprecated.** Enable moving the `event.name` attribute (captured by one of the other mechanisms of capturing attributes) to the log event name. | | ||
|
|
||
| The `otel.event.name` key is supported in key-value pairs (SLF4J 2.x fluent API). When present, its value is used as the log event name and is not emitted as an attribute. |
There was a problem hiding this comment.
Besides key-value pairs and mdc we previously also captured event name from logstash.
There was a problem hiding this comment.
oh, thanks! added logstash support
laurit
left a comment
There was a problem hiding this comment.
As far as I understand it will be possible to emit events only from logback. What are the users of other logging libraries supposed to do? Use otel apis? But otel logging api wasn't supposed to be used by users, has that changed?
Also log4j2 |
I couldn't find where it would do that. I think the event name part is missing for log4j2 structured messages (didn't try, just looked at the code, maybe missed it somehow). We might need to add a test with slf4j2 structured logging and log4j logger to check what that does. |
oh, I understand now, yeah, looks like that requires supporting ThreadContext on the log4j side. and then it's hard to justify not supporting MDC, will probably go that route... |
93dceba to
f0eed51
Compare
| // Event name priority (last writer wins): KVP > MDC > structured args > logstash markers | ||
| // > logger context. Sources are called in ascending priority order. |
There was a problem hiding this comment.
sorry for the reordering in this method, this is why
| } | ||
|
|
||
| @ParameterizedTest | ||
| @MethodSource("eventNameProperties") |
There was a problem hiding this comment.
could consider
@ValueSource(strings = {"event.name", "otel.event.name"})
| // copied from EventIncubatingAttributes | ||
| private static final AttributeKey<String> EVENT_NAME = AttributeKey.stringKey("event.name"); | ||
|
|
||
| private static final String OTEL_EVENT_NAME_KEY = "otel.event.name"; |
There was a problem hiding this comment.
wondering whether there is also going to be a semconv constant for this? If so could consider typing it to AttributeKey<String>
otel.event.nameis stable now!The current
event.namemapping is behind an experimental config option, so we can drop that in the (minor) release following the minor release where this PR lands.