Skip to content
This repository was archived by the owner on Nov 20, 2025. It is now read-only.

feat: add debug logging support#1898

Closed
feywind wants to merge 4 commits intogoogleapis:mainfrom
feywind:add-debug-logging
Closed

feat: add debug logging support#1898
feywind wants to merge 4 commits intogoogleapis:mainfrom
feywind:add-debug-logging

Conversation

@feywind
Copy link
Contributor

@feywind feywind commented Nov 22, 2024

Adds debug logging support for auth. This needs a bit more work/testing still.

It's also waiting on this to finish:
googleapis/gax-nodejs#1669

@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Nov 22, 2024
@feywind feywind marked this pull request as ready for review December 10, 2024 18:42
@feywind feywind requested a review from a team December 10, 2024 18:42
@feywind feywind requested a review from a team as a code owner December 10, 2024 18:42
@feywind
Copy link
Contributor Author

feywind commented Dec 10, 2024

I need to update this for when the debug logger piece is officially reviewed and released, and there's a system test thing to look at, but the code itself is largely ready, I think.

Comment on lines +23 to +24
import {log as makeLog} from 'google-logging-utils';
const log = makeLog('auth');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may be cleaner and flexible to add this to the base AuthClient class and have it as a member or static method.

This will allow:

  • Customers to change/modify the log functionality (if they want)
  • Dedupe the import statements across the files

Additionally, in the next major we're planning to unify the Transporter instance into AuthClient via this PR:

It would be an even smaller migration after that has landed as we could extend the GaxiosOptions interface with a Symbol (which would note the type of log) and use log.info on every request (as all requests would use that _request method).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick review. I'll make that change tomorrow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of the logging can be done through AuthClient now, good call. There are a few objects that don't seem to derive ultimately from AuthClient, so I moved makeLog() into the class itself to make it cleaner. (makeLog caches the loggers by name, so there's little harm in calling it multiple times.)

@feywind feywind added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 20, 2024
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Dec 20, 2024
@d-goog
Copy link
Contributor

d-goog commented Feb 21, 2025

Is this version of the PR safe to close?

@feywind
Copy link
Contributor Author

feywind commented Mar 4, 2025

Yep, sorry, I seem to have forgotten to close it.

@feywind feywind closed this Mar 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants