-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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(compute/metadata): add debug logging #11078
base: main
Are you sure you want to change the base?
Conversation
Requires googleapis/gax-go#380 |
"net" | ||
"net/http" | ||
"net/url" | ||
"os" | ||
"strings" | ||
"sync" | ||
"time" | ||
|
||
"github.com/googleapis/gax-go/v2/internallog" |
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.
is there any way to avoid this? compute/metadata is impossible to avoid as a dependency (google.golang.org/grpc pulls it in, etc)
we really should not be forcing transitive deps on cloud modules like this
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.
At this layer specifically we cloud likely just take slog as a dep and pass in a logger. But this means the default case can never work with the semantics we are adding here.
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.
compute/metadata specifically is required by almost everything in the go ecosystem, keeping its dependency graph small and avoiding transitive deps to other cloud libraries is much more important for this module than anything else in this repo.
using stdlib slog or a locally-defined interface and pushing callers to wire any specific cloud instrumentation they want is much more in line with what I would expect here.
I'd also like to see a presubmit that will catch / guard against unconsidered introduction of new dependencies to this module in particular
No description provided.