-
Notifications
You must be signed in to change notification settings - Fork 29
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
Conversation
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
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 Report
@@ 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
Continue to review full report at Codecov.
|
src/main/java/com/google/cloud/logging/logback/TraceLoggingEventEnhancer.java
Outdated
Show resolved
Hide resolved
src/main/java/com/google/cloud/logging/logback/TraceLoggingEventEnhancer.java
Outdated
Show resolved
Hide resolved
…ntEnhancer.java Co-authored-by: yoshi-code-bot <70984784+yoshi-code-bot@users.noreply.github.com>
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.
Also, LGTM
🤖 I have created a release \*beep\* \*boop\* --- ## [0.119.0](https://www.github.com/googleapis/java-logging-logback/compare/v0.118.7...v0.119.0) (2020-12-08) ### Features * adds support to group logs of the same http request ([#257](https://www.github.com/googleapis/java-logging-logback/issues/257)) ([d98051c](https://www.github.com/googleapis/java-logging-logback/commit/d98051c969de254c128a92f330175bc085a75855)) ### Bug Fixes * logback logging exceptions with null message ([#251](https://www.github.com/googleapis/java-logging-logback/issues/251)) ([d9856f6](https://www.github.com/googleapis/java-logging-logback/commit/d9856f6c88458648f81612fc8b15315abcec9683)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).
@simonz130 / @chingor13 just to clarify, this means that I need to put
in an interceptor that process every request that I get? Thanks, |
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); |
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.
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?
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.
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.
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.
@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?
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:
Then use grouping by traceId in Cloud Logging:
Fixes #94 ☕️