-
Notifications
You must be signed in to change notification settings - Fork 637
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
Changes from 7 commits
701a1fd
054e73a
21b4757
e558a65
722195f
8b9dedb
7ab3bcc
0d9d39f
726edeb
5834fd4
52d19aa
0c3be5c
d7f823c
f2133c2
b48ea28
8c711b3
15c21c0
50779f1
533c3f1
12f5032
ff55bad
f974587
cb627af
d8593cd
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 |
|---|---|---|
|
|
@@ -141,7 +141,7 @@ def substituted_because_contains_sensitive_data(cls) -> "AnnotatedValue": | |
| from collections.abc import Container, MutableMapping, Sequence | ||
| from datetime import datetime | ||
| from types import TracebackType | ||
| from typing import Any, Callable, Dict, Mapping, NotRequired, Optional, Type | ||
| from typing import Any, Callable, Dict, List, Mapping, NotRequired, Optional, Type | ||
|
|
||
| from typing_extensions import Literal, TypedDict | ||
|
|
||
|
|
@@ -152,6 +152,65 @@ class SDKInfo(TypedDict): | |
| version: str | ||
| packages: "Sequence[Mapping[str, str]]" | ||
|
|
||
| class KeyValueCollectionBehaviour(TypedDict): | ||
| mode: 'Literal["off", "deny_list", "allow_list"]' | ||
|
ericapisani marked this conversation as resolved.
Outdated
|
||
| terms: "NotRequired[List[str]]" | ||
|
|
||
| class GenAICollectionUserOptions(TypedDict, total=False): | ||
| inputs: bool | ||
| outputs: bool | ||
|
|
||
| class GenAICollectionBehaviour(TypedDict): | ||
| inputs: bool | ||
| outputs: bool | ||
|
|
||
| class GraphQLCollectionUserOptions(TypedDict, total=False): | ||
| document: bool | ||
| variables: bool | ||
|
|
||
| class GraphQLCollectionBehaviour(TypedDict): | ||
| document: bool | ||
| variables: bool | ||
|
|
||
| class DatabaseCollectionUserOptions(TypedDict, total=False): | ||
| query_params: bool | ||
|
|
||
| class DatabaseCollectionBehaviour(TypedDict): | ||
| query_params: bool | ||
|
|
||
| class HttpHeadersCollectionUserOptions(TypedDict, total=False): | ||
| request: "KeyValueCollectionBehaviour" | ||
| response: "KeyValueCollectionBehaviour" | ||
|
|
||
| class HttpHeadersCollectionBehaviour(TypedDict): | ||
| request: "KeyValueCollectionBehaviour" | ||
| response: "KeyValueCollectionBehaviour" | ||
|
|
||
| class DataCollectionUserOptions(TypedDict, total=False): | ||
|
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. For my understanding, we have these type pairs ( I see why we have it like this but I'm afraid the two might drift eventually because we forget to update one of them. It's not like I have better ideas 😀 The complexity we introduce with the config needs to be dealt with someplace, and I guess this is an ok place.
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.
Yup, you've got it! I wanted to spare us from all the extra code we'd have to write to satisfy the typechecker when performing the checks within the integrations 😅 I agree with you about the risk of drift. I thought it'd be worth it in this case because after the data collection spec is implemented and settled, the amount of updates we'd need to do here would likely be minimal. If we think of anything better throughout the implementation of the spec though, happy to come back and change the approach! 😄 |
||
| user_info: bool | ||
| cookies: "KeyValueCollectionBehaviour" | ||
| http_headers: "HttpHeadersCollectionUserOptions" | ||
| http_bodies: "List[str]" | ||
| query_params: "KeyValueCollectionBehaviour" | ||
| graphql: "GraphQLCollectionUserOptions" | ||
| gen_ai: "GenAICollectionUserOptions" | ||
| database: "DatabaseCollectionUserOptions" | ||
| stack_frame_variables: bool | ||
| frame_context_lines: int | ||
|
|
||
| class DataCollection(TypedDict): | ||
| provided_by_user: bool | ||
| user_info: bool | ||
| cookies: "KeyValueCollectionBehaviour" | ||
| http_headers: "HttpHeadersCollectionBehaviour" | ||
| http_bodies: "List[str]" | ||
| query_params: "KeyValueCollectionBehaviour" | ||
| graphql: "GraphQLCollectionBehaviour" | ||
| gen_ai: "GenAICollectionBehaviour" | ||
| database: "DatabaseCollectionBehaviour" | ||
| stack_frame_variables: bool | ||
| frame_context_lines: int | ||
|
|
||
| # "critical" is an alias of "fatal" recognized by Relay | ||
| LogLevelStr = Literal["fatal", "critical", "error", "warning", "info", "debug"] | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,10 @@ | |
| VERSION, | ||
| ClientConstructor, | ||
| ) | ||
| from sentry_sdk.data_collection import ( | ||
|
sentry-warden[bot] marked this conversation as resolved.
|
||
| _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 | ||
|
ericapisani marked this conversation as resolved.
Outdated
|
||
|
|
||
|
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"] | ||
|
ericapisani marked this conversation as resolved.
Outdated
|
||
|
|
||
| @property | ||
| def dsn(self) -> "Optional[str]": | ||
| """Returns the configured DSN as string.""" | ||
|
|
||


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