Skip to content

Support HTTP response body if sendDefaultPii and maxResponseBodySize are enabled #624

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

Closed
Tracked by #41
marcellodesales opened this issue Oct 23, 2021 · 11 comments
Closed
Tracked by #41

Comments

@marcellodesales
Copy link

Requirement

  • We'd like to have full visibility on the logs from the app
  • We need to print the full HTTP Request/Response submitted to Sentry
  • For whitelisting dozens of apps, it's impossible to double-check the configs for all of them
    • We need ta way to verify how the apps are configured

Design

  • We have mobile apps: Flutter (Android, iOS)
  • We have Web apps: Flutter (Progressive Web Apps)
  • We have backend services: Java, Golang, Python, Node.js

We need a consistent way to turn HTTP requests on/off

@marandaneto
Copy link
Contributor

@marcellodesales thanks for reporting.

not sure if I follow the feature, can you describe the use case?
do you mean when an error happens (eg 500) or every single request even if 200?

See https://docs.sentry.io/platforms/dart/usage/advanced-usage/

The SentryHttpClient class has configurations like captureFailedRequests, can be used along with failedRequestStatusCodes and maxRequestBodySize, does that work?

we don't log response since it could contain PII and it could be quite a huge payload, events cannot be higher than 1MB.

in case none of them work, you can always implement your own HTTP Client calling the Sentry APIs manually eg Sentry.captureEvent(x), Sentry.captureException(x), etc...

@marandaneto
Copy link
Contributor

@marcellodesales any news here? thanks

@marcellodesales
Copy link
Author

@marandaneto Sorry I couldn't comment on this earlier... I think it's reasonable to implement it manually as you described since logging may contain PII info for the body, but I don't think it applies for the rest of the HTTP request (verb, headers) when connecting to the sentry servers.

Before I close the ticket, do you think you could support in the basic implementation the option to turn on logging the HTTP request/response from sentry without the body? Everything we are submitting to sentry would be captured... The reason why I'm asking is because we use HTTP Traces with a given specific TransactionID that actually gets to Sentry dashboard as a tag. However, in our systems should be able to track that the specific transactionID was submitted to Sentry...

Anyway, let me know if that's possible...
thank you

@marandaneto
Copy link
Contributor

We do collect some data if the sendDefaultPii is enabled (including headers, verb, request body), see https://github.com/getsentry/sentry-dart/blob/main/dart/lib/src/http_client/failed_request_client.dart#L162-L173
Unfortunately, we can't collect the request and response payload, that's best if you implement your own, you could copy+paste our solution and customize it, I hope that helps.

I'll discuss if the response body would be possible too, but likely not, thanks for clarifying.

@marandaneto marandaneto moved this to Needs Discussion in [DEPRECATED] Mobile SDKs Dec 21, 2021
@marandaneto marandaneto changed the title HTTP Request/Response loggers Support HTTP response body if sendDefaultPii and maxResponseBodySize are enabled Dec 21, 2021
@marandaneto marandaneto moved this from Needs Discussion to Backlog in [DEPRECATED] Mobile SDKs Jan 13, 2022
@marandaneto
Copy link
Contributor

When this is done, we should actually also add to the develop docs https://develop.sentry.dev/sdk/features/#attaching-request-body-in-server-sdks
This feature does make sense for the other SDKs as well, We'll create issues for the other SDKs to support maxResponseBodySize.

@bruno-garcia
Copy link
Member

bruno-garcia commented Jan 13, 2022

Probably best to bring this into the TSC given this setting only exists in server SDKs afaik. I missed the fact this is named differently. But still best we bring this up to a larger team as this could be put in all SDKs

@philipphofmann
Copy link
Member

I added it to the next TSC, @bruno-garcia, and @marandaneto.

@ueman
Copy link
Collaborator

ueman commented Jul 6, 2022

Related: getsentry/develop#401

@marandaneto
Copy link
Contributor

This is blocked because it requires a RFC, more context here

@marandaneto
Copy link
Contributor

Depends on getsentry/team-mobile#41

@marandaneto marandaneto moved this from Blocked to Backlog in [DEPRECATED] Mobile SDKs Jun 22, 2023
@marandaneto
Copy link
Contributor

marandaneto commented Jul 10, 2023

maxRequestBodySize already exists, sets as Request#data, defaults to never.
Add maxResponseBodySize, sets as SentryResponse#data, defaults to never.

This can be done on the integrations for the packages http and dio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

7 participants