chore(Pubsub): Add adhoc debug logging#32136
chore(Pubsub): Add adhoc debug logging#32136cy-yun wants to merge 10 commits intogoogleapis:mainfrom
Conversation
7c8937c to
2ead334
Compare
feywind
left a comment
There was a problem hiding this comment.
Looks fine from a logging perspective, just wondering about the histogram class comment.
|
The acceptance tests pass locally. They can be flakey when running on Kokoro. Acceptance test failures are unrelated to my changes. |
f36a306 to
4ebacf2
Compare
039d065 to
bdd5d68
Compare
There was a problem hiding this comment.
I had a change to look into this a bit more. I think the approach we want to take it is along the lines of:
- Allow configuration of logger in
lib/google/cloud/pubsub.rb. I was thinking something like here, with 3 options. - Once we have it there, we can pass it to
Serviceto use downstream. - Logs should happen via
Messagefromgoogle-logging-utils:
@logger&.info do
# This block is only executed if INFO level is enabled, making it efficient.
Google::Logging::Message.new(....)
end
This logic should be able to handle instances of a default Ruby logger (e.g. stdin/out) or Google::Logging::StructuredFormatter, which you can use for testing.
HI @aandreassa . Thanks for your feedback.
You can programmatically configure a custom logger: require "google/cloud/pubsub"
require "logger"
# Configure a logger for the pubsub library
Google::Cloud.configure.pubsub.logger = Logger.new "my-app.log"If the custom logger is not configured, it will default to a standard stdout logger.
Talk tomorrow! |
73e205a to
1990c5e
Compare
|
These two pull requests introduce a standardized and dynamically configurable debug logging framework for Google Cloud Ruby clients and implement it in the Pub/Sub library. The primary goal was to enable on-demand debug logging that could be controlled by the developer via configuring a custom logger and controlling an environment variable to turn logs on/off. This required passing a logger from the handwritten client (google-cloud-pubsub) down to the underlying GAPIC client (google-cloud-pubsub-v1). I needed to build a wrapper around the logger (creating a logger duck-type) that will evaluate the environment variable before emitting logs. The Solution:
Once the first 2 PRs are approved, I will create a new PR that passes the GoogleSdkLoggerDelegator down to the GAPIC layer (google-cloud-pubsub-v1). |
1990c5e to
38f281b
Compare
viacheslav-rostovtsev
left a comment
There was a problem hiding this comment.
Let's split this PR in two: one that is concerned with introducing a new debug logger (and its tests) and one that is using it in specific flows.
| module PubSub | ||
| ## | ||
| # @private | ||
| class Logging |
There was a problem hiding this comment.
Class name should be a noun
https://docs.google.com/document/d/1R5UNVksMGCsN-KOthICr8-McrLmlN9F0XP99AKp6plY/edit?tab=t.s19hlulorz61
https://buganizer.corp.google.com/issues/427197812