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

Report usage of .save and StripeClient #1612

Merged
merged 8 commits into from
Dec 1, 2023
Merged

Conversation

richardm-stripe
Copy link
Contributor

@richardm-stripe richardm-stripe commented Nov 27, 2023

Changelog

  • Reports uses of the deprecated .save and of StripeClient in X-Stripe-Client-Telemetry. (You can disable telemetry via \Stripe\Stripe::setEnableTelemetry(false);, see the README.)

Details

I've added usage to several infrastructure methods:

  • Several underscored methods in lib/ApiOperations/Request.php. (Since they are underscored, they should not be considered part of the public interface). I've included a default value there for expedience sake, though. Most callsites for these methods have no usage to report.
  • request and requestStream in ApiRequestor. These are public, but I have added a default value so that it is non-breaking.
  • The private _requestRaw and _requestRawStreaming in ApiRequestor. No default value here, but they are private.
  • The RequestTelemetry constructor which is public (but I doubt it has much external use), but I've added a default value there.
  • I've modified BaseStripeClient to always pass in through ["stripe_client"] as usage to ApiRequestor, and modified ->save in ApiOperations to pass in ["save"] through to the helpers in Request.php.

Testing

I modified the existing test case for telemetry to use a method that would trigger "save" usage to be included. It mocks at the HTTP Client level, which is fine, because all the logic for telemetry is thankfully implemented at a higher level of abstraction in ApiRequestor.

@richardm-stripe richardm-stripe marked this pull request as ready for review November 29, 2023 20:51
@richardm-stripe richardm-stripe merged commit be56918 into master Dec 1, 2023
22 checks passed
@richardm-stripe richardm-stripe deleted the richardm-usage branch December 1, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants