Skip to content

Conversation

@tpetracca
Copy link
Contributor

We're seeing instances of trace information disappearing from logs on a thread that previously had traceIds. Trying to track down where/how they're getting cleared.

==COMMIT_MSG==
debug logging when clearing trace
==COMMIT_MSG==

@tpetracca tpetracca requested a review from carterkozak January 20, 2021 19:01
@changelog-app

This comment has been minimized.

@policy-bot policy-bot bot requested a review from ferozco January 20, 2021 19:01
}

private static void logClearingTrace() {
log.debug("Clearing current trace", SafeArg.of("maybeTrace", Optional.ofNullable(currentTrace.get())));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Let's wrap this with log.isDebugEnabled to prevent optional and safeArg allocations when debug logging is disabled.

note: no reason to use an Optional for the safe-arg, as far as the logging system is concerned empty optional and null are indistinguishable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're here, thoughts on mirroring the debug line after a traceId is set? That way we get a full sense of the scope transitions.

Copy link
Contributor

@carterkozak carterkozak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof, I never submitted comments. Sorry for the delay!

Approving, change looks good to me. Up to you whether you'd like to take action on the nits.

@tpetracca tpetracca force-pushed the tp/log-on-trace-clear branch from f41e2e7 to f3ec634 Compare January 21, 2021 19:15
@tpetracca tpetracca merged commit f4bb698 into develop Jan 21, 2021
@tpetracca tpetracca deleted the tp/log-on-trace-clear branch January 21, 2021 20:38
@svc-autorelease
Copy link
Collaborator

Released 4.11.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants