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

feat: adds support to group logs of the same http request #257

Merged
merged 4 commits into from Dec 8, 2020

Conversation

simonz130
Copy link

@simonz130 simonz130 commented Dec 4, 2020

With TraceLoggingEventEnhancer users can set the traceId of the incoming request and then all the logs that will be created on the same thread will have the traceId property set to that traceId. This allows Cloud Logging to group log entries from the same request in the UI for easier troubleshooting.

Example:

  1. One time configuration (adding logging event enhancer to logs appender):
LoggingAppender cloudAppender = new LoggingAppender();
cloudAppender.addLoggingEventEnhancer("com.google.cloud.logging.logback.TraceLoggingEventEnhancer");
  1. User's code (when receiving http request):
TraceLoggingEventEnhancer.setCurrentTraceId(
      "projects/" + GOOGLE_PROJECT_ID + "/traces/" + traceId);
  1. Then just logging regularly:
LOGGER.info("XXX");

Then use grouping by traceId in Cloud Logging:
image

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #94 ☕️

With TraceLoggingEventEnhancer users can set the traceId of the incoming request and then all the logs that will be created on the same thread will have the traceId property set to that traceId. This allows Cloud Logging to group log entries from the same request in the UI for easier troubleshooting
@simonz130 simonz130 added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. api: logging Issues related to the googleapis/java-logging-logback API. labels Dec 4, 2020
@simonz130 simonz130 requested a review from a team as a code owner December 4, 2020 21:01
@simonz130 simonz130 self-assigned this Dec 4, 2020
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Dec 4, 2020
With TraceLoggingEventEnhancer users can set the traceId of the incoming request and then all the logs that will be created on the same thread will have the traceId property set to that traceId. This allows Cloud Logging to group log entries from the same request in the UI for easier troubleshooting
@codecov
Copy link

codecov bot commented Dec 4, 2020

Codecov Report

Merging #257 (899f68e) into master (5ad9f24) will increase coverage by 0.57%.
The diff coverage is 77.77%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #257      +/-   ##
============================================
+ Coverage     68.05%   68.62%   +0.57%     
- Complexity       35       39       +4     
============================================
  Files             2        3       +1     
  Lines           144      153       +9     
  Branches         15       17       +2     
============================================
+ Hits             98      105       +7     
  Misses           33       33              
- Partials         13       15       +2     
Impacted Files Coverage Δ Complexity Δ
...oud/logging/logback/TraceLoggingEventEnhancer.java 77.77% <77.77%> (ø) 4.00 <4.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ad9f24...899f68e. Read the comment docs.

…ntEnhancer.java

Co-authored-by: yoshi-code-bot <70984784+yoshi-code-bot@users.noreply.github.com>
@simonz130 simonz130 merged commit d98051c into master Dec 8, 2020
@simonz130 simonz130 deleted the traceLogGroupping branch December 8, 2020 19:58
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

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

Also, LGTM

@darewreck54
Copy link

@simonz130 / @chingor13 just to clarify, this means that I need to put

TraceLoggingEnhancer.setCurrentTraceId(
      "projects/" + GOOGLE_PROJECT_ID + "/traces/" + traceId);

in an interceptor that process every request that I get?

Thanks,
Derek

@darewreck54
Copy link

I'm still seeing the request thread TraceLoggingEnhancer trace id = null with upgrading to the 1.119.0-alpha. Perhaps i'm doing it wrong. I'm still seeing the log thread having the traceId as null while the request filter trace id being stored as mentioned in your previous thread. @simonz130

Object value = e.getMDCPropertyMap().get(TRACE_ID);
String traceId = value != null ? value.toString() : null;
if (traceId != null) {
builder.setTrace(traceId);

Choose a reason for hiding this comment

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

Isn't it suppose to be configured adding a label for stack driver to recognize it

   builder.addLabel("trace_id", traceId);

https://cloud.google.com/logging/docs/setup/java#flexible_environment?

Copy link
Author

Choose a reason for hiding this comment

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

Hello @darewreck54, I realized that my PR description was not accurate - it didn't include instructions to add logging event appender for logback to use:

LoggingAppender cloudAppender = new LoggingAppender();
cloudAppender.addLoggingEventEnhancer("com.google.cloud.logging.logback.TraceLoggingEventEnhancer");

I just tested the code you provided earlier to illustrate what you wanted to achieve with request grouping, with the new feature introduced in this PR and it worked for me on AppEngine Flex.
We will update official documentation to include info about request logs grouping and I updated this PR's description.

Choose a reason for hiding this comment

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

@simonz130 I had another question. Is this library meant to be used service deployed in GKE? I noticed the recommended way is to pipe logs to stdout, but this approach pipes it to a file. https://cloud.google.com/solutions/best-practices-for-operating-containers#use_the_native_logging_mechanisms_of_containers

Any suggestion from that stand front?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the googleapis/java-logging-logback API. cla: yes This human has signed the Contributor License Agreement. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logging with google-cloud-logging-logback is not grouping request logs
5 participants