Repository navigation
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 8 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): | ||
|
Collaborator
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 ( | ||
|
Check warning on line 26 in sentry_sdk/client.py
|
||
|
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 @@ | |
| 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"] | ||
|
Check warning on line 357 in sentry_sdk/client.py
|
||
| ) | ||
|
|
||
| if rv["socket_options"] and not isinstance(rv["socket_options"], list): | ||
|
|
@@ -386,6 +391,12 @@ | |
| # 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 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 @@ | |
| 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) | ||
|
|
||
|
|
@@ -721,9 +749,17 @@ | |
| .. versionadded:: 2.0.0 | ||
|
|
||
| Returns whether the client should send default PII (Personally Identifiable Information) data to Sentry. | ||
| """ | ||
| return self.options.get("send_default_pii") or False | ||
|
Check warning on line 753 in sentry_sdk/client.py
|
||
|
|
||
| @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.