Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MdcLoggingDirectives (withMdcLogging, withMdcEntry, withMdcEntries, extractMarkerLog) #3974
base: main
Are you sure you want to change the base?
MdcLoggingDirectives (withMdcLogging, withMdcEntry, withMdcEntries, extractMarkerLog) #3974
Changes from 1 commit
65eb272
629938f
6f6c4e0
d434e00
776f4ef
c652021
d0ac46e
efb56e0
e2f0da6
17c6540
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LL 27 to 35 were purely type tetris on my part. They work, but I'm not 100% sure I setup the logger the right way. Very open to input here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need that, aren't all logger also
MarkerLoggingAdapter
s anyway even withoutwithMdcLogging
enabled?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. I'm assuming you're implying we could just use
extractLog
instead of having this.extractLog
returns aLoggingAdapter
:akka-http/akka-http/src/main/scala/akka/http/scaladsl/server/directives/BasicDirectives.scala
Line 279 in 90b5ecb
MarkerLoggingAdapter
has a few extra methods, e.g., https://github.com/akka/akka/blob/main/akka-actor/src/main/scala/akka/event/Logging.scala#L1848-L1849Maybe it's true that the
LoggingAdapter
that gets returned fromextractLog
happens to always also extendMarkerLoggingAdapter
, but that's not specified in the return type, so we can't access those methods.