Skip to content
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

Retry stats default documented incorrectly #3059

Closed
ssigwart opened this issue Jan 14, 2025 · 2 comments
Closed

Retry stats default documented incorrectly #3059

ssigwart opened this issue Jan 14, 2025 · 2 comments
Assignees
Labels
documentation This is a problem with documentation.

Comments

@ssigwart
Copy link

Describe the issue

On https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/guide_configuration.html#config-stats, for stats.retries, it says "Retry statistics are collected by default and returned."

That doesn't seem to be the case. I think the following code shows that the default for stats is false.

'stats' => [
'type' => 'value',
'valid' => ['bool', 'array'],
'default' => false,
'doc' => 'Set to true to gather transfer statistics on requests sent. Alternatively, you can provide an associative array with the following keys: retries: (bool) Set to false to disable reporting on retries attempted; http: (bool) Set to true to enable collecting statistics from lower level HTTP adapters (e.g., values returned in GuzzleHttp\TransferStats). HTTP handlers must support an http_stats_receiver option for this to have an effect; timer: (bool) Set to true to enable a command timer that reports the total wall clock time spent on an operation in seconds.',
'fn' => [__CLASS__, '_apply_stats'],
],

Therefore, I think the following code sets stats.retries to false.

$defaults = array_fill_keys(
['http', 'retries', 'timer'],
$value === true
);
$args['stats'] = is_array($value)
? array_replace($defaults, $value)
: $defaults;

Links

https://docs.aws.amazon.com/sdk-for-php/v3/developer-guide/guide_configuration.html#config-stats

@ssigwart ssigwart added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Jan 14, 2025
@stobrien89 stobrien89 self-assigned this Mar 6, 2025
@stobrien89 stobrien89 removed the needs-triage This issue or PR still needs to be triaged. label Mar 6, 2025
@stobrien89
Copy link
Member

Hi @ssigwart,

Apologies for the confusion. I've contacted our documentation team to get the description for the retries sub-option of stats updated.

You are correct in that stats defaults to false, however, the retries description is technically correct— retries in that context is a sub-option of stats and by default it is set to true when stats is enabled. That's what is happening in the referenced code (lines 911-914 of ClientResolver.php). However, I think it would be less confusing to document retries like this:

retries (bool)
Set to false to disable reporting on retries attempted. When stats is enabled, retry statistics are collected and returned by default.

Closing for now, but let us know if you have any additional questions.

Copy link

github-actions bot commented Mar 6, 2025

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation This is a problem with documentation.
Projects
None yet
Development

No branches or pull requests

2 participants