-
Notifications
You must be signed in to change notification settings - Fork 388
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 debug logging support #1903
Conversation
Hello! With recent enhancements in the library we may be able to greatly streamline this PR by adding a listener here (thanks to the removal of Transporter):
Every auth request goes through this function. Let me know if you’d like to sync on this or have any questions. |
@d-goog Ah, lovely. Thanks! |
@@ -107,8 +109,10 @@ export class UserRefreshClient extends OAuth2Client { | |||
refresh_token: this._refreshToken, | |||
target_audience: targetAudience, | |||
} as {}), | |||
}); | |||
}; | |||
AuthClient.setMethodName(opts, 'fetchIdToken'); |
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.
Completely optional: Since these classes extend AuthClient
we could use the class's setMethodName
and avoid importing AuthClient
.
AuthClient.setMethodName(opts, 'fetchIdToken'); | |
UserRefreshClient.setMethodName(opts, 'fetchIdToken'); |
Here and in other places
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.
Actually, after thinking about it, I'm not a super fan of static inheritance like that. I think it makes it more obscure what's actually being called. (Also, several of the other usages were not deriving from anything that had setMethodName
so the usage was mixed.) I'm okay with it either way, but maybe leaning toward not changing it.
Adds debug logging support for auth.
It's also waiting on this to finish:
googleapis/gax-nodejs#1669
This is a duplicate of another PR to make the GitHub bot macguffins go.