-
Notifications
You must be signed in to change notification settings - Fork 450
feat(BigQuery): Add logging functionality to BigQuery #8790
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
base: main
Are you sure you want to change the base?
Conversation
| return new RequestWrapper($config); | ||
| } | ||
|
|
||
| private function getRetryListener(): callable |
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.
Can we add a php doc explaining what this function does? e.g. "The retry listener will get called in {@see ClassName} on retry. We need to supply a custom listener so that the "retryAttempt" is (whatever happens to it) by (whatever does that thing)"
This would help me as well because I am confused by the retryAttempt option - it doesn't appear to be a header, and it doesn't seem to show up anywhere else. Is it just a "call option" that is only for logging purposes?
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.
You are correct, I think using the header terminology was the incorrect one. let me fix that.
It is literally just a call option for logging purposes that the retryMiddleware sets for the logging logic to have this value.
| $logger->debug( | ||
| Argument::that(function (string $jsonString) use (&$retryHeaderAppeared) { | ||
| $jsonParsed = json_decode($jsonString, true); | ||
| if (isset($jsonParsed['jsonPayload']['retryAttempt'])) { |
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.
I am confused because retryAttempt does not appear to be a header here, it's part of the JSON payload.
I assume that's ok, I am just confused by the wording of "header"
|
Note: |
This PR aims to add the logging feature to the Big Query handwritten library.