Ensure JobClusterActor's logs always contain the clusterName - #870
Open
sileshidev-lab wants to merge 1 commit into
Open
Ensure JobClusterActor's logs always contain the clusterName#870sileshidev-lab wants to merge 1 commit into
sileshidev-lab wants to merge 1 commit into
Conversation
Log lines emitted from JobClusterActor didn't consistently include
the cluster name, making it hard to trace behavior of a specific
cluster in the logs. Manually passing `name` into all ~256 log call
sites in this class would be tedious and error-prone (easy to miss
new call sites going forward).
Instead, override aroundReceive (which every message this actor
processes passes through) to set the cluster name in SLF4J's MDC
before delegating to the actual message handler, and clear it
afterward. This makes the cluster name automatically available to
every log line emitted while handling a message, for logging
backends that render MDC values (structured/JSON encoders, or a
pattern layout with %X{clusterName}).
Added a test that exercises aroundReceive directly via TestActorRef
and asserts MDC contains the cluster name during message handling
and is cleared afterward.
Fixes Netflix#385
sileshidev-lab
requested review from
Andyz26,
calvin681,
dtrager02,
fdc-ntflx,
hellolittlej and
james-lubin
as code owners
August 5, 2026 14:23
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes #385 — log lines emitted from
JobClusterActordidn't consistently include the cluster name, making it hard to trace a specific cluster's behavior in logs (~256 log call sites in this class, added ad-hoc over time, many withoutname).Approach: rather than manually retrofitting every existing call site (tedious, easy to miss, and doesn't guard against new call sites forgetting it later), this overrides
aroundReceive— the single method every message this actor processes passes through — to set the cluster name in SLF4J's MDC before delegating to the actual handler, and clear it in afinallyblock afterward. This makes the cluster name automatically available to every log line emitted anywhere during that message's handling, for any logging backend that renders MDC values (structured/JSON encoders, or a pattern layout with%X{clusterName}).Verified
AbstractActorWithTimers.aroundReceive(via theTimerstrait) already delegates further up the chain toAbstractActor.aroundReceive, so callingsuper.aroundReceive(receive, msg)from the override preserves existing timer bookkeeping — confirmed by inspecting the actual Akka 2.6.15akka-actorjar's bytecode rather than assuming the API shape.Test plan
testAroundReceivePutsClusterNameInMdcDuringMessageHandlingAndClearsItAfterinJobClusterAkkaTest. Since MDC is thread-local, it exercisesaroundReceivedirectly viaTestActorRef(bypassing the mailbox/dispatcher) with a receive handler that captures the MDC value synchronously during "processing," then asserts it matches the cluster name and is cleared afterward.akkaTesttask (this class's actual test task — this project splits*AkkaTestclasses into a separate Gradle task fromtest): 58 passed (57 pre-existing + 1 new), 0 regressionstesttask for the module: 358 passed, 0 regressionscompileJavaclean, no new warnings on the modified file (verified against a stashed baseline)