-
Notifications
You must be signed in to change notification settings - Fork 636
feat(data-collection): create DataCollection option in client #6702
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
701a1fd
054e73a
21b4757
e558a65
722195f
8b9dedb
7ab3bcc
0d9d39f
726edeb
5834fd4
52d19aa
0c3be5c
d7f823c
f2133c2
b48ea28
8c711b3
15c21c0
50779f1
533c3f1
12f5032
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,10 @@ | |
| VERSION, | ||
| ClientConstructor, | ||
| ) | ||
| from sentry_sdk.data_collection import ( | ||
| _map_from_send_default_pii, | ||
| resolve_data_collection, | ||
| ) | ||
| from sentry_sdk.envelope import Envelope, Item, PayloadRef | ||
| from sentry_sdk.integrations import _DEFAULT_INTEGRATIONS, setup_integrations | ||
| from sentry_sdk.integrations.dedupe import DedupeIntegration | ||
|
|
@@ -70,6 +74,7 @@ | |
| from sentry_sdk._log_batcher import LogBatcher | ||
| from sentry_sdk._metrics_batcher import MetricsBatcher | ||
| from sentry_sdk._types import ( | ||
| DataCollection, | ||
| Event, | ||
| EventDataCategory, | ||
| Hint, | ||
|
|
@@ -345,11 +350,11 @@ def _get_options(*args: "Optional[str]", **kwargs: "Any") -> "Dict[str, Any]": | |
| if rv["enable_tracing"] is True and rv["traces_sample_rate"] is None: | ||
| rv["traces_sample_rate"] = 1.0 | ||
|
|
||
| rv["data_collection"] = resolve_data_collection(rv) | ||
|
|
||
| if rv["event_scrubber"] is None: | ||
| rv["event_scrubber"] = EventScrubber( | ||
| send_default_pii=( | ||
| False if rv["send_default_pii"] is None else rv["send_default_pii"] | ||
| ) | ||
| send_default_pii=rv["data_collection"]["user_info"] | ||
| ) | ||
|
|
||
| if rv["socket_options"] and not isinstance(rv["socket_options"], list): | ||
|
|
@@ -386,6 +391,12 @@ def _get_options(*args: "Optional[str]", **kwargs: "Any") -> "Dict[str, Any]": | |
| # Older Python versions | ||
| module_not_found_error = ImportError # type: ignore | ||
|
|
||
| _DISABLED_DATA_COLLECTION_CONFIG = _map_from_send_default_pii( | ||
| send_default_pii=False, | ||
| include_local_variables=True, | ||
| include_source_context=True, | ||
| ) | ||
|
|
||
|
|
||
| class BaseClient: | ||
| """ | ||
|
|
@@ -425,6 +436,10 @@ def parsed_dsn(self) -> "Optional[Dsn]": | |
| def should_send_default_pii(self) -> bool: | ||
| return False | ||
|
|
||
| @property | ||
| def data_collection(self) -> "DataCollection": | ||
| return _DISABLED_DATA_COLLECTION_CONFIG | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to add this accessor?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might not depending on our convo in the other comment that's related. This was added for consistency with |
||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unpickled client missing data_collectionLow Severity
Additional Locations (2)Reviewed by Cursor Bugbot for commit 15c21c0. Configure here. |
||
| def is_active(self) -> bool: | ||
| """ | ||
| .. versionadded:: 2.0.0 | ||
|
|
@@ -614,6 +629,19 @@ def _record_lost_event( | |
| self.options["error_sampler"] = sample_all | ||
| self.options["traces_sampler"] = sample_all | ||
| self.options["profiles_sampler"] = sample_all | ||
| # data_collection was resolved in _get_options() before this | ||
| # spotlight override flipped send_default_pii on. Re-derive it so | ||
| # data_collection agrees with should_send_default_pii() in | ||
| # DSN-less spotlight mode (only when the user did not set | ||
| # data_collection explicitly). | ||
| if not self.options["data_collection"]["provided_by_user"]: | ||
| self.options["data_collection"] = _map_from_send_default_pii( | ||
| send_default_pii=True, | ||
| include_local_variables=self.options["include_local_variables"] | ||
| is not False, | ||
| include_source_context=self.options["include_source_context"] | ||
| is not False, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spotlight frame flags disagreeMedium Severity In DSN-less Spotlight mode, re-derived Reviewed by Cursor Bugbot for commit 15c21c0. Configure here. |
||
| ) | ||
|
ericapisani marked this conversation as resolved.
|
||
|
|
||
| self.session_flusher = SessionFlusher(capture_func=_capture_envelope) | ||
|
|
||
|
|
@@ -724,6 +752,14 @@ def should_send_default_pii(self) -> bool: | |
| """ | ||
| return self.options.get("send_default_pii") or False | ||
|
|
||
| @property | ||
|
ericapisani marked this conversation as resolved.
Outdated
|
||
| def data_collection(self) -> "DataCollection": | ||
| """ | ||
| Returns the resolved :class:`~sentry_sdk.data_collection.DataCollection` | ||
| config for this client. | ||
| """ | ||
| return self.options["data_collection"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to add this accessor?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not, but had added it to be consistent with
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's also client options for which we do not provide accessors. We can always add the accessor later on if there's popular demand, but IMO it'd be good to avoid public API bloat.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, will remove |
||
|
|
||
| @property | ||
| def dsn(self) -> "Optional[str]": | ||
| """Returns the configured DSN as string.""" | ||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.