feat: add logging to the supported transports #585
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bshaffer
changed the title
Hectorhammett
added
the
do not merge
Hectorhammett
commented
Sep 26, 2024
Hectorhammett
force-pushed
the
logging
branch
from
8846ceb to
573214b
Compare
Hectorhammett
force-pushed
the
logging
branch
from
54833af to
8a84fdd
Compare
bshaffer
requested changes
Dec 4, 2024
There was a problem hiding this comment.
A few things I've observed when debugging this:
- As we are logging the response code separately, it should go before the rest of the response log
- We do NOT want to log all the client confiugration we are currently logging (see this example). We should only log the configuration supplied by the developer
- The order of the logs ideally would match the order of the requests being made. Right now, the request to the services is logged first, followed by the auth request. In reality, the auth request is made first. If this is really hard to implement, the way we have it is okay, but it's worth considering.
Hectorhammett
force-pushed
the
logging
branch
from
3ded4a0 to
a99b928
Compare
bshaffer
reviewed
Dec 9, 2024
Hectorhammett
force-pushed
the
logging
branch
from
98ebe80 to
240a125
Compare
bshaffer
reviewed
Dec 10, 2024
bshaffer
reviewed
Dec 10, 2024
bshaffer
reviewed
Dec 11, 2024
Hectorhammett
force-pushed
the
logging
branch
from
ac098af to
90e16a2
Compare
Hectorhammett
removed
the
do not merge
bshaffer
approved these changes
Dec 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
related: googleapis/google-auth-library-php#578
Adds logging capabilities to the Gapic Clients by adding logging into the supported transports (Http, Grpc and GrpcFallback).