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

GH-5208 Enable request tracing through logs using new associated request id #5209

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

benherber
Copy link

GitHub issue resolved: #5208
Briefly describe the changes proposed in this PR:

This PR adds a unique request identifier to MDC in order to allow for more intelligent log parsing and aggregation as well as aid in server debugging.


PR Author Checklist (see the contributor guidelines for more details):

  • my pull request is self-contained
  • I've added tests for the changes I made
  • I've applied code formatting (you can use mvn process-resources to format from the command line)
  • I've squashed my commits where necessary
  • every commit message starts with the issue number (GH-xxxx) followed by a meaningful description of the change

private volatile String origThreadName;

@Override
public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler)
throws Exception {
origThreadName = Thread.currentThread().getName();
Thread.currentThread().setName(getThreadName());
MDC.put(REQUEST_ID, UUID.randomUUID().toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

I've also used MDC for this in other dev projects. It's probably still the best option.

From experience it can get quite expensive to generate a lot of UUIDs especially if the transactions are very small. Can we have the UUID as a string at startup and then an AtomicLong that we increment and append?

Copy link
Author

@benherber benherber Nov 29, 2024

Choose a reason for hiding this comment

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

Good point! Should be changed. Introduced a process UUID which is postpended with an AtomicLong in a format which should be parsable by some tooling if need be.

Let me know your thoughts.

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.

Add Notion of Request ID to Make Logging More Transparent
3 participants