-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(issues): infer project platform if not set #95402
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
Open
cvxluo
wants to merge
19
commits into
master
Choose a base branch
from
cvxluo/infer-project-platform
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+61
−1
Open
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
36b63c7
add helper to set project platform
lobsterkatie 88faf1b
infer project platform if not set
cvxluo 6fb23e5
stop inferring project if changed manually
cvxluo afc8907
fix tests
cvxluo c559e38
handle 'other' events
cvxluo 2d2ec4d
clean up typos + tests
cvxluo c7c5a2a
Merge branch 'master' into cvxluo/infer-project-platform
cvxluo 05c536d
add lock + atomic transaction
cvxluo 597b3c4
stop inferring to 'other' + options gating + filter out sample events
cvxluo 9bfc4ba
clean up option key
cvxluo 29f6f79
wrap refresh in transaction
cvxluo 1a95c89
adjust cache key
cvxluo 1e25c0a
use queryset update instead of transaction
cvxluo b0e8ec8
use VALID_PLATFORMS instead of ad-hoc event platform filtering
cvxluo 26c7fe3
save changes
cvxluo 7bde68e
better filtering
cvxluo 054c0b2
fix query
cvxluo 89d40a7
fix comment
cvxluo 553895b
remove extra lock + cache
cvxluo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,13 +14,14 @@ | |
from django.core.cache import cache | ||
from django.core.exceptions import ValidationError | ||
from django.db import IntegrityError, OperationalError, connection, router, transaction | ||
from django.db.models import Max | ||
from django.db.models import Max, Q | ||
from django.db.models.signals import post_save | ||
from django.utils.encoding import force_str | ||
from urllib3.exceptions import MaxRetryError, TimeoutError | ||
from usageaccountant import UsageUnit | ||
|
||
from sentry import ( | ||
audit_log, | ||
eventstore, | ||
eventstream, | ||
eventtypes, | ||
|
@@ -37,6 +38,7 @@ | |
LOG_LEVELS_MAP, | ||
MAX_TAG_VALUE_LENGTH, | ||
PLACEHOLDER_EVENT_TITLES, | ||
VALID_PLATFORMS, | ||
DataCategory, | ||
InsightModules, | ||
) | ||
|
@@ -127,6 +129,7 @@ | |
from sentry.types.group import GroupSubStatus, PriorityLevel | ||
from sentry.usage_accountant import record | ||
from sentry.utils import metrics | ||
from sentry.utils.audit import create_system_audit_entry | ||
from sentry.utils.cache import cache_key_for_event | ||
from sentry.utils.circuit_breaker import ( | ||
ERROR_COUNT_CACHE_KEY, | ||
|
@@ -137,6 +140,7 @@ | |
from sentry.utils.event import has_event_minified_stack_trace, has_stacktrace, is_handled | ||
from sentry.utils.eventuser import EventUser | ||
from sentry.utils.metrics import MutableTags | ||
from sentry.utils.options import sample_modulo | ||
from sentry.utils.outcomes import Outcome, track_outcome | ||
from sentry.utils.projectflags import set_project_flag_and_signal | ||
from sentry.utils.safe import get_path, safe_execute, setdefault_path, trim | ||
|
@@ -464,6 +468,11 @@ def save( | |
# After calling _pull_out_data we get some keys in the job like the platform | ||
_pull_out_data([job], projects) | ||
|
||
# Sometimes projects get created without a platform (e.g. through the API), in which case we | ||
# attempt to set it based on the first event | ||
if sample_modulo("sentry:infer_project_platform", project.id): | ||
_set_project_platform_if_needed(project, job["event"]) | ||
|
||
event_type = self._data.get("type") | ||
if event_type == "transaction": | ||
job["data"]["project"] = project.id | ||
|
@@ -690,6 +699,26 @@ def _pull_out_data(jobs: Sequence[Job], projects: ProjectsMapping) -> None: | |
job["groups"] = [] | ||
|
||
|
||
def _set_project_platform_if_needed(project: Project, event: Event) -> None: | ||
if project.platform: | ||
return | ||
|
||
if event.platform not in VALID_PLATFORMS or event.get_tag("sample_event") == "yes": | ||
return | ||
|
||
updated = Project.objects.filter( | ||
Q(id=project.id) & (Q(platform__isnull=True) | Q(platform="")) | ||
).update(platform=event.platform) | ||
|
||
if updated: | ||
create_system_audit_entry( | ||
organization=project.organization, | ||
target_object=project.id, | ||
event=audit_log.get_event_id("PROJECT_EDIT"), | ||
data={**project.get_audit_log_data(), "platform": event.platform}, | ||
) | ||
Comment on lines
+709
to
+719
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. Might be worth try/catch around this so that if anything fails we don't crash out |
||
|
||
|
||
@sentry_sdk.tracing.trace | ||
def _get_or_create_release_many(jobs: Sequence[Job], projects: ProjectsMapping) -> None: | ||
for job in jobs: | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
from sentry.testutils.cases import TestCase | ||
from sentry.testutils.helpers import override_options | ||
from sentry.testutils.helpers.eventprocessing import save_new_event | ||
|
||
|
||
class ProjectPlatformInferTest(TestCase): | ||
@override_options({"sentry:infer_project_platform": 1.0}) | ||
def test_platform_inferred_on_event(self): | ||
project = self.create_project() | ||
|
||
save_new_event({"message": "test", "platform": "javascript"}, project) | ||
|
||
project.refresh_from_db() | ||
assert project.platform == "javascript" | ||
|
||
@override_options({"sentry:infer_project_platform": 1.0}) | ||
def test_platform_does_not_override_existing_platform(self): | ||
project = self.create_project(platform="python") | ||
|
||
save_new_event({"message": "test", "platform": "javascript"}, project) | ||
|
||
project.refresh_from_db() | ||
assert project.platform == "python" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Could combine this into the same
if