Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Capture log4j2 context data (mdc) attributes #4959

Merged
merged 6 commits into from
Dec 22, 2021
Merged

Capture log4j2 context data (mdc) attributes #4959

merged 6 commits into from
Dec 22, 2021

Conversation

trask
Copy link
Member

@trask trask commented Dec 21, 2021

Adds otel.instrumentation.log4j-appender.experimental-log-attributes property that enables capturing log4j2 context data as attributes.

Adds otel.instrumentation.log4j-appender.experimental.capture-context-data-attributes property which is an allow-list of context data attributes to capture. It accepts a lone * to mean capture all context data attributes.

Context data attributes xyz is captured as log4j.context_data.xyz, taking some inspiration from http.request.header.xyz.

@anuraaga
Copy link
Contributor

anuraaga commented Dec 21, 2021

I would add the same allow-list param as request headers, or otherwise a deny-list if it seems more appropriate for log context. allow-list seems to fit well IMO with the expectation that context data only does something if it is actually referenced in e.g. log4j.xml pattern string. I have seen log context be used for very non-log stuff such as database sharding keys (worse is putting the actual user ID in and generating the key at query-time which I've also seen...).

@trask
Copy link
Member Author

trask commented Dec 21, 2021

I think capturing all MDC attributes is the nicest default (once spec'd), under the assumption that most people put things into logging MDC because they want those things tied to their logs.

Definitely agree with supporting people who put weird things into logging MDC as well though, whether its via allow-list or deny-list.

I'd like to get a sense if others agree with the (eventual) default above, as that will help influence decision on allow-list vs deny-list.

@trask trask marked this pull request as ready for review December 21, 2021 07:04
@trask
Copy link
Member Author

trask commented Dec 22, 2021

Switched to allow-list, see updated description.

@trask trask merged commit 5bc64a1 into open-telemetry:main Dec 22, 2021
@trask trask deleted the capture-log4j2-context-data-attributes branch December 22, 2021 18:16
RashmiRam pushed a commit to RashmiRam/opentelemetry-auto-instr-java that referenced this pull request May 23, 2022
* Capture log4j2 context data (mdc) attributes

* Spotless

* Remove system.out.println

* Switch to allow-list

* Spotless
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants