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

Allow merging user data on Scope #929

Closed
volkyeth opened this issue Dec 9, 2019 · 2 comments
Closed

Allow merging user data on Scope #929

volkyeth opened this issue Dec 9, 2019 · 2 comments
Milestone

Comments

@volkyeth
Copy link
Contributor

volkyeth commented Dec 9, 2019

Summary

Currently the Scope only allows us to replace user data with the setUser() method, and because of this it's not possible to gradually include more specific user data. It would be good to merge that data.

Motivation

We don't always get all the user info from the same place, so it would be useful to gradually populate that data. For instance: adding the IP before the user is authenticated, then adding username and email afterwards.

Additional Context

The motivation for this request is the fact that the RequestListener from getsentry/sentry-symfony is overwriting my user data. As it does add some useful data, it would be best if it was possible to keep the user data it provided while also adding some data myself without replacing it.

@ste93cry
Copy link
Collaborator

ste93cry commented Dec 9, 2019

Indeed it makes sense and in fact the Unified API documentation says that the setUser method of the scope should do a shallow merge, so +1 on fixing this. How to do such change is another discussion unfortunately, as modifying the behavior of an existing function may break existing applications: adding a boolean argument defaulting to false that let users decide whether to replace the data as a whole or to merge existing data with the new one should be fine. Of course, we should throw a deprecation for the old behavior and then in 3.0 we will align with what other SDKs do out of the box. We should also add a removeUser method to let users remove information from the context, or we should clear all data if an empty array is passed (documentation talks about these two ways for solving this, but personally I find the first one the clearest). If this makes sense to you, are you willing to contribute the changes?

@volkyeth
Copy link
Contributor Author

There you go @ste93cry: #931

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

No branches or pull requests

2 participants