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

Add util to send user feedback #1064

Closed
ThiefMaster opened this issue Mar 24, 2021 · 7 comments
Closed

Add util to send user feedback #1064

ThiefMaster opened this issue Mar 24, 2021 · 7 comments
Labels
enhancement New feature or request triaged Has been looked at recently during old issue triage

Comments

@ThiefMaster
Copy link

Applications which don't already use the client-side sentry SDK may want to display their own UI to send user feedback to associate with an error that was logged to sentry.

It would be nice if the user feedback api was exposed in a clean way. For example, it should automatically infer the correct host/path from the DSN.

Not sure if this doesn't actually require changes on Sentry itself... right now I don't see how one would get the data needed for organization_slug and project_slug using just the DSN...

@antonpirker
Copy link
Member

Have a method in class Dsn to expose the user feedback api url.

@antonpirker antonpirker added triaged Has been looked at recently during old issue triage Good first issue Good for newcomers Help wanted Extra attention is needed Hacktoberfest 🎃 labels Aug 28, 2023
@vagi8
Copy link
Contributor

vagi8 commented Sep 2, 2023

Hey I am planning to take it up. I have taken a look at the Dsn class in utils but I was unable to find a method for feedback api url. Nor I could find any organization / project slug values in the Dsn nor in Auth. What am I missing ?

@antonpirker
Copy link
Member

Hey @vagi8

You are right. Right now the SDK only knows the DSN but not the organization_slug and project_slug. I looked at how the browser JS SDK does it [1] and it seems it creates an envelope.

So I guess this is not a minor change as we thought. We will need to discuss this internally if we want to have this feature in the Python SDK.

1: https://github.com/getsentry/sentry-javascript/blob/f63036ba6d896f624328c857d2170f8a6843f5e1/packages/browser/src/client.ts#L97-L109

@antonpirker antonpirker removed Good first issue Good for newcomers Help wanted Extra attention is needed Hacktoberfest 🎃 labels Sep 11, 2023
@antonpirker
Copy link
Member

We discussed this internally and because this is not only about giving the user the Feddback api URL, but implementing a feature that puts user feedback in an envelope and send it to Sentry, we will not implement this right now or in the short to mid term future. We have more pressing things on our agenda. So I will close this issue.

@theCapypara
Copy link

theCapypara commented Oct 11, 2023

@antonpirker Would this still be OK to contribute as a PR, or is this a feature you currently don't want implemented at all for some reason?

I'm not sure what you mean by "not only about giving the user the Feddback api URL, but implementing a feature that puts user feedback in an envelope and send it to Sentry". Isn't this also what this issue was originally about? Exposing the user feedback API via the SDK so developers can show their own forms to the users and then the SDK will send it to Sentry via the user-feedback api?

I would be interested in contributing such a feature, but I'm not sure what you mean about the envelope, the only thing I can find is the official API documentation (https://docs.sentry.io/api/projects/submit-user-feedback/) which I thought would be the one to use?

@theCapypara
Copy link

theCapypara commented Oct 11, 2023

Nevermind, found info on the topic: https://develop.sentry.dev/sdk/envelopes/

Would a PR that does things similar to getsentry/sentry-javascript#7729 be OK to submit?

I see the Python SDK seems to construct envelopes a bit different, but what I'm thinking is, add a function similiar to what sentry_sdk.add_context or sentry_sdk.set_extra etc. do, and mark the user feedback at the Scope, then extend Scope.apply_to_event to add it to the Event and then in capture_event make sure the envelope path is chose and the user feedback is added to the envelope. Would that be roughly the correct way to do it?

Actually never mind, I guess we would need to send the envelope on it's own, since we need to send it after the event was already sent, since you usually have to ask the user for feedback after the event happened. But this should still be possible without major refactoring in the SDK via the transport's capture_envelope or not? And as the event ID we would use the last event ID (maybe with the option to override?)

The docs say:

User Feedbacks / Reports can be ingested separately from their events. We recommended to send them in the same Envelope.

So sending them seperately seems fine and appropriate in this generic use case IMO and it seems to be the same thing the JavaScript SDK does.


Unless I'm missing something there doesn't seem to be any big refactoring required or anything else in the SDK that blocks this. Everything needed seems to be there.

theCapypara added a commit to theCapypara/sentry-python that referenced this issue Oct 13, 2023
This adds an API to the Sentry Python SDK that captures user feedback
via envelope.

This is implemented very similiarly to how it is done for the
JavaScript SDK,
see getsentry/sentry-javascript#7729.

Fixes getsentryGH-1064
theCapypara added a commit to theCapypara/sentry-python that referenced this issue Oct 30, 2023
This adds an API to the Sentry Python SDK that captures user feedback
via envelope.

This is implemented very similiarly to how it is done for the
JavaScript SDK,
see getsentry/sentry-javascript#7729.

Fixes getsentryGH-1064
@theCapypara
Copy link

I opened a PR to implement this: #2442

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request triaged Has been looked at recently during old issue triage
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants