-
Notifications
You must be signed in to change notification settings - Fork 38
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: Add support for library instrumentation #979
Conversation
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.
I did not find instrumentation for the calls to Logging.Write()
and Logging.WriteAsync()
. Are they expected to be in a separate PR? I recommend to instrument all paths before merging.
google-cloud-logging/src/main/java/com/google/cloud/logging/Instrumentation.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/Instrumentation.java
Outdated
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/Instrumentation.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/Instrumentation.java
Show resolved
Hide resolved
google-cloud-logging/src/main/java/com/google/cloud/logging/LoggingHandler.java
Outdated
Show resolved
Hide resolved
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.
there is a case of NPE that should be handled. Left few more comments for considerations. Beside that it is good to go.
This feature provides an ability to log extra entry with diagnostics structure which contains logging library information. Such entry is logged only once when first entry is written by a process using logging library.
Fixes #978, 751 🦕