-
Notifications
You must be signed in to change notification settings - Fork 361
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
Add ability to get raw request / response for audit log #73
Comments
Could you elaborate on why our dashboard logs are not sufficient for your use case? In terms of stripe-java: I'm a bit nervous to include the full request object in the Anyway, I'm happy to entertain this. Would I be able to convince you to submit a pull request? I'll also note that short of rolling your own SDK, you'd in general always be welcome to fork stripe-java to add the request / response logging that you need. You wouldn't need to roll your own SDK. Let me know your thoughts. If you're not up for taking a stab at adding what you need, could you describe the desired interface in a bit more detail? Would you expect that any fetched |
Don't get me wrong - the Stripe dashboard log is outstanding and we've used it several times. The reason we need the logging "in-house" is 1. what if the call doesn't make it to stripe (network issue) - then your log can't show it, 2. we use multiple external APIs and track all calls to all APIs (including some internal stuff), 3. with all logging "internal" we're able to attach all sorts of reference information to each log entry - I can look up a user and see precisely what the sequence of API calls were, which helps a lot when trying to answer "why doesn't it work?" The information can't really be put in the StripeObject returned by e.g. Charge.create() - there are several cases that require logging where such an object is not returned (e.g. the failure cases). The strategy I've seen used by other SDKs is to not use a static method, but instead have a Client object created which handles the request and then returns the StripeObject if succesful, but regardless it holds on to the most recent request information (since separate Client objects can be created by separate threads, this can easily be used in a thread safe manner by the calling code). So instead of StripeClient client = new StripeClient(apiKey);
try {
client.charge(params);
} finally {
doLogging(client); // client now has e.g. lastRequest, lastResponse, lastStatus, etc
} (specific implementations may do things slightly differently, but you get the general gist) Under the hood, I realize that this would be a significant departure from the current structure, at least under the hood. Perhaps it would be an easier change to not to a StripeClient, but instead do a StripeCall, encapsulating a single call to Stripe. The usage would then be something like: StripeCall call = Charge.createCall(params, apiKey);
try {
call.execute();
} finally {
doLogging(call); // call now has e.g. lastRequest, lastResponse, lastStatus, etc
} That would unfortunately duplicate the entire static method set, which is not super-exciting. It's not easy to retrofit in a both clean and backwards compatible manner on the current SDK. Thoughts? |
I think the correct approach here would be to abstract all connection logic into pluggable backends. You could then write a custom backend that extends the provided backend to write all requests / responses to your logging backend. |
I'm going to close this issue for now, but tag it with |
This issue has been added to the Version 2 Roadmap https://github.com/stripe/stripe-java/wiki/Roadmap |
has there been any activity on this? |
For auditing purposes we need to log every single call made to Stripe, and precisely what was sent and what was received back (yes, I'm aware of your corresponding log, that is unfortunately not sufficient here).
Currently it is not possible to get that information - as soon as _request returns it is dropped on the floor: https://github.com/stripe/stripe-java/blob/master/src/main/java/com/stripe/net/APIResource.java#L418
It would be great if that capability could be added - would be unfortunate to have to roll our own SDK "just" to get that.
Thanks!
The text was updated successfully, but these errors were encountered: