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

feat: merging & splitting issues #26612

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
39 changes: 18 additions & 21 deletions posthog/api/error_tracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@

from posthog.api.forbid_destroy_model import ForbidDestroyModel
from posthog.api.routing import TeamAndOrgViewSetMixin
from posthog.models import ErrorTrackingSymbolSet
from posthog.models.error_tracking import ErrorTrackingStackFrame
from posthog.api.utils import action
from posthog.models.error_tracking import ErrorTrackingIssue, ErrorTrackingSymbolSet, ErrorTrackingStackFrame
from posthog.models.utils import uuid7
from posthog.storage import object_storage

Expand All @@ -28,29 +28,26 @@ class ObjectStorageUnavailable(Exception):
pass


# class ErrorTrackingGroupSerializer(serializers.ModelSerializer):
# class Meta:
# model = ErrorTrackingGroup
# fields = ["assignee", "status"]
class ErrorTrackingIssueSerializer(serializers.ModelSerializer):
class Meta:
model = ErrorTrackingIssue
fields = ["assignee", "status"]


# class ErrorTrackingGroupViewSet(TeamAndOrgViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet):
# scope_object = "INTERNAL"
# queryset = ErrorTrackingGroup.objects.all()
# serializer_class = ErrorTrackingGroupSerializer
class ErrorTrackingGroupViewSet(TeamAndOrgViewSetMixin, ForbidDestroyModel, viewsets.ModelViewSet):
scope_object = "INTERNAL"
queryset = ErrorTrackingIssue.objects.all()
serializer_class = ErrorTrackingIssueSerializer

# def safely_get_object(self, queryset) -> QuerySet:
# stringified_fingerprint = self.kwargs["pk"]
# fingerprint = json.loads(urlsafe_base64_decode(stringified_fingerprint))
# group, _ = queryset.get_or_create(fingerprint=fingerprint, team=self.team)
# return group
def safely_get_queryset(self, queryset):
return queryset.filter(team_id=self.team.id)

# @action(methods=["POST"], detail=True)
# def merge(self, request, **kwargs):
# group: ErrorTrackingGroup = self.get_object()
# merging_fingerprints: list[list[str]] = request.data.get("merging_fingerprints", [])
# group.merge(merging_fingerprints)
# return Response({"success": True})
@action(methods=["POST"], detail=True)
def merge(self, request, **kwargs):
issue: ErrorTrackingIssue = self.get_object()
ids: list[str] = request.data.get("ids", [])
issue.merge(issue_ids=ids)
return Response({"success": True})


class ErrorTrackingStackFrameSerializer(serializers.ModelSerializer):
Expand Down
8 changes: 7 additions & 1 deletion posthog/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@
from .element import Element
from .element_group import ElementGroup
from .entity import Entity
from .error_tracking import ErrorTrackingIssue, ErrorTrackingStackFrame, ErrorTrackingSymbolSet
from .error_tracking import (
ErrorTrackingIssue,
ErrorTrackingIssueFingerprintV2,
ErrorTrackingStackFrame,
ErrorTrackingSymbolSet,
)
from .event.event import Event
from .event_buffer import EventBuffer
from .event_definition import EventDefinition
Expand Down Expand Up @@ -102,6 +107,7 @@
"ElementGroup",
"Entity",
"ErrorTrackingIssue",
"ErrorTrackingIssueFingerprintV2",
"ErrorTrackingStackFrame",
"ErrorTrackingSymbolSet",
"Event",
Expand Down
118 changes: 85 additions & 33 deletions posthog/models/error_tracking/error_tracking.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
from django.db import models
from django.db import models, transaction
from django.contrib.postgres.fields import ArrayField

from posthog.models.utils import UUIDModel
from posthog.models.team import Team
from posthog.models.user import User
from django.db import transaction
from django.db.models import Q, QuerySet
from posthog.models.error_tracking.sql import INSERT_ERROR_TRACKING_ISSUE_FINGERPRINT_OVERRIDES

from posthog.kafka_client.client import ClickhouseProducer
from posthog.kafka_client.topics import KAFKA_ERROR_TRACKING_ISSUE_FINGERPRINT


class ErrorTrackingIssue(UUIDModel):
Expand All @@ -20,6 +23,37 @@ class Status(models.TextChoices):
name = models.TextField(null=True, blank=True)
description = models.TextField(null=True, blank=True)

def merge(self, issue_ids: list[str]) -> None:
override_inserts: list[tuple[str, int, str]] = []

with transaction.atomic():
for issue_id in issue_ids:
fingerprints = resolve_fingerprints_for_issue(team_id=self.team.pk, issue_id=issue_id)

for fingerprint in fingerprints:
override_inserts.append(
update_error_tracking_issue_fingerprint(
team_id=self.team.pk, issue_id=self.id, fingerprint=fingerprint
)
)

ErrorTrackingIssue.objects.filter(team=self.team, id__in=issue_ids).delete()

update_error_tracking_issue_fingerprint_overrides(team_id=self.team.pk, override_inserts=override_inserts)

def split(self, fingerprints: list[str]) -> None:
override_inserts: list[tuple[str, int, str]] = []

for fingerprint in fingerprints:
new_issue = ErrorTrackingIssue.objects.create(team=self.team)
override_inserts.append(
update_error_tracking_issue_fingerprint(
team_id=self.team.pk, issue_id=new_issue.id, fingerprint=fingerprint
)
)

update_error_tracking_issue_fingerprint_overrides(team_id=self.team.pk, override_inserts=override_inserts)


class ErrorTrackingIssueAssignment(UUIDModel):
issue = models.ForeignKey(ErrorTrackingIssue, on_delete=models.CASCADE)
Expand Down Expand Up @@ -114,36 +148,6 @@ class Status(models.TextChoices):
blank=True,
)

@classmethod
def filter_fingerprints(cls, queryset, fingerprints: list[list]) -> QuerySet:
query = Q(fingerprint__in=fingerprints)

for fp in fingerprints:
query |= Q(merged_fingerprints__contains=fp)

return queryset.filter(query)

@transaction.atomic
def merge(self, fingerprints: list[list[str]]) -> None:
if not fingerprints:
return

# sets don't like lists so we're converting fingerprints to tuples
def convert_fingerprints_to_tuples(fps: list[list[str]]):
return [tuple(f) for f in fps]

merged_fingerprints = set(convert_fingerprints_to_tuples(self.merged_fingerprints))
merged_fingerprints.update(convert_fingerprints_to_tuples(fingerprints))

merging_groups = ErrorTrackingGroup.objects.filter(team=self.team, fingerprint__in=fingerprints)
for group in merging_groups:
merged_fingerprints |= set(convert_fingerprints_to_tuples(group.merged_fingerprints))

merging_groups.delete()
# converting back to list of lists before saving
self.merged_fingerprints = [list(f) for f in merged_fingerprints]
self.save()


# DEPRECATED: Use ErrorTrackingIssueFingerprintV2 instead
class ErrorTrackingIssueFingerprint(models.Model):
Expand All @@ -155,3 +159,51 @@ class ErrorTrackingIssueFingerprint(models.Model):

class Meta:
constraints = [models.UniqueConstraint(fields=["team", "fingerprint"], name="unique fingerprint for team")]


def resolve_fingerprints_for_issue(team_id: int, issue_id: str) -> list[str]:
override_records = ErrorTrackingIssueFingerprintV2.objects.filter(team_id=team_id, issue_id=issue_id)
return [r.fingerprint for r in override_records]


def update_error_tracking_issue_fingerprint(team_id: int, issue_id: str, fingerprint: str) -> tuple[str, int, str]:
issue_fingerprint = ErrorTrackingIssueFingerprintV2.objects.select_for_update().get(
team_id=team_id, fingerprint=fingerprint
)
issue_fingerprint.issue_id = issue_id
issue_fingerprint.version = (issue_fingerprint.version or 0) + 1
issue_fingerprint.save(update_fields=["version", "issue_id"])

return (fingerprint, issue_fingerprint.version, issue_id)


def update_error_tracking_issue_fingerprint_overrides(
team_id: int, override_inserts: list[tuple[str, int, str]]
) -> None:
for fingerprint, version, issue_id in override_inserts:
override_error_tracking_issue_fingerprint(
team_id=team_id, fingerprint=fingerprint, issue_id=issue_id, version=version
)


def override_error_tracking_issue_fingerprint(
team_id: int,
fingerprint: str,
issue_id: str,
version=0,
is_deleted: bool = False,
sync: bool = False,
) -> None:
p = ClickhouseProducer()
p.produce(
topic=KAFKA_ERROR_TRACKING_ISSUE_FINGERPRINT,
sql=INSERT_ERROR_TRACKING_ISSUE_FINGERPRINT_OVERRIDES,
data={
"team_id": team_id,
"fingerprint": fingerprint,
"issue_id": issue_id,
"version": version,
"is_deleted": int(is_deleted),
},
sync=sync,
)
4 changes: 4 additions & 0 deletions posthog/models/error_tracking/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,7 @@
TRUNCATE_ERROR_TRACKING_ISSUE_FINGERPRINT_OVERRIDES_TABLE_SQL = (
f"TRUNCATE TABLE IF EXISTS {ERROR_TRACKING_ISSUE_FINGERPRINT_OVERRIDES_TABLE} ON CLUSTER '{CLICKHOUSE_CLUSTER}'"
)

INSERT_ERROR_TRACKING_ISSUE_FINGERPRINT_OVERRIDES = """
INSERT INTO error_tracking_issue_fingerprint_overrides (fingerprint, issue_id, team_id, is_deleted, version, _timestamp, _offset, _partition) SELECT %(fingerprint)s, %(issue_id)s, %(team_id)s, %(is_deleted)s, %(version)s, now(), 0, 0 VALUES
"""
136 changes: 76 additions & 60 deletions posthog/models/error_tracking/test/test_error_tracking.py
Original file line number Diff line number Diff line change
@@ -1,60 +1,76 @@
# class TestErrorTracking(BaseTest):
# def test_defaults(self):
# group = ErrorTrackingGroup.objects.create(status="active", team=self.team, fingerprint=["a_fingerprint"])

# assert group.fingerprint == ["a_fingerprint"]
# assert group.merged_fingerprints == []
# assert group.assignee is None

# def test_filtering(self):
# ErrorTrackingGroup.objects.bulk_create(
# [
# ErrorTrackingGroup(team=self.team, fingerprint=["first_error"]),
# ErrorTrackingGroup(
# team=self.team, fingerprint=["second_error"], merged_fingerprints=[["previously_merged"]]
# ),
# ErrorTrackingGroup(team=self.team, fingerprint=["third_error"]),
# ]
# )

# matching_groups = ErrorTrackingGroup.objects.filter(fingerprint__in=[["first_error"], ["second_error"]])
# assert matching_groups.count() == 2

# matching_groups = ErrorTrackingGroup.objects.filter(merged_fingerprints__contains=["previously_merged"])
# assert matching_groups.count() == 1

# matching_groups = ErrorTrackingGroup.filter_fingerprints(
# queryset=ErrorTrackingGroup.objects, fingerprints=[["first_error"], ["previously_merged"]]
# )
# assert matching_groups.count() == 2

# def test_merge(self):
# primary_group = ErrorTrackingGroup.objects.create(
# status="active",
# team=self.team,
# fingerprint=["a_fingerprint"],
# merged_fingerprints=[["already_merged_fingerprint"]],
# )
# merge_group_1 = ErrorTrackingGroup.objects.create(
# status="active", team=self.team, fingerprint=["new_fingerprint"]
# )
# merge_group_2 = ErrorTrackingGroup.objects.create(
# status="active",
# team=self.team,
# fingerprint=["another_fingerprint"],
# merged_fingerprints=[["merged_fingerprint"]],
# )

# merging_fingerprints = [merge_group_1.fingerprint, merge_group_2.fingerprint, ["no_group_fingerprint"]]
# primary_group.merge(merging_fingerprints)

# assert sorted(primary_group.merged_fingerprints) == [
# ["already_merged_fingerprint"],
# ["another_fingerprint"],
# ["merged_fingerprint"],
# ["new_fingerprint"],
# ["no_group_fingerprint"],
# ]

# # deletes the old groups
# assert ErrorTrackingGroup.objects.count() == 1
from posthog.test.base import BaseTest
from posthog.models.error_tracking import ErrorTrackingIssue, ErrorTrackingIssueFingerprintV2


class TestErrorTracking(BaseTest):
def create_issue(self, fingerprints) -> ErrorTrackingIssue:
issue = ErrorTrackingIssue.objects.create(team=self.team)
for fingerprint in fingerprints:
ErrorTrackingIssueFingerprintV2.objects.create(team=self.team, issue=issue, fingerprint=fingerprint)
return issue

def test_defaults(self):
issue = ErrorTrackingIssue.objects.create(team=self.team)

assert issue.status == "active"
assert issue.name is None

def test_basic_merge(self):
issue_one = self.create_issue(["fingerprint_one"])
issue_two = self.create_issue(["fingerprint_two"])

issue_two.merge(issue_ids=[issue_one.id])

# remaps the first fingerprint to the second issue
assert ErrorTrackingIssueFingerprintV2.objects.filter(issue_id=issue_two.id).count() == 2
# bumps the version
override = ErrorTrackingIssueFingerprintV2.objects.filter(fingerprint="fingerprint_one").first()
assert override
assert override.version == 1

# deletes issue one
assert ErrorTrackingIssue.objects.count() == 1

def test_merge_multiple_times(self):
issue_one = self.create_issue(["fingerprint_one"])
issue_two = self.create_issue(["fingerprint_two"])
issue_three = self.create_issue(["fingerprint_three"])

issue_two.merge(issue_ids=[issue_one.id])
issue_three.merge(issue_ids=[issue_two.id])

# only the third issue remains
assert ErrorTrackingIssue.objects.count() == 1
# all fingerprints point to the third issue
assert ErrorTrackingIssueFingerprintV2.objects.filter(issue_id=issue_three.id).count() == 3

# bumps versions of the merged issues correct number of times
override = ErrorTrackingIssueFingerprintV2.objects.filter(fingerprint="fingerprint_one").first()
assert override
assert override.version == 2
override = ErrorTrackingIssueFingerprintV2.objects.filter(fingerprint="fingerprint_two").first()
assert override
assert override.version == 1

def test_merging_multiple_issues_at_once(self):
issue_one = self.create_issue(["fingerprint_one"])
issue_two = self.create_issue(["fingerprint_two"])
issue_three = self.create_issue(["fingerprint_three"])

issue_three.merge(issue_ids=[issue_one.id, issue_two.id])

assert ErrorTrackingIssueFingerprintV2.objects.filter(issue_id=issue_three.id).count() == 3

def test_splitting_fingerprints(self):
issue = self.create_issue(["fingerprint_one", "fingerprint_two", "fingerprint_three"])

issue.split(fingerprints=["fingerprint_one", "fingerprint_two"])

# creates two new issues
assert ErrorTrackingIssue.objects.count() == 3

# bumps the version but no longer points to the old issue
override = ErrorTrackingIssueFingerprintV2.objects.filter(fingerprint="fingerprint_one").first()
assert override
assert override.issue_id != issue.id
assert override.version == 1
2 changes: 2 additions & 0 deletions posthog/models/team/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ def delete_bulky_postgres_data(team_ids: list[int]):
from posthog.models.feature_flag.feature_flag import FeatureFlagHashKeyOverride
from posthog.models.insight_caching_state import InsightCachingState
from posthog.models.person import Person, PersonDistinctId
from posthog.models.error_tracking import ErrorTrackingIssueFingerprintV2
from posthog.models.early_access_feature import EarlyAccessFeature

_raw_delete(EarlyAccessFeature.objects.filter(team_id__in=team_ids))
_raw_delete(PersonDistinctId.objects.filter(team_id__in=team_ids))
_raw_delete(ErrorTrackingIssueFingerprintV2.objects.filter(team_id__in=team_ids))
_raw_delete(CohortPeople.objects.filter(cohort__team_id__in=team_ids))
_raw_delete(FeatureFlagHashKeyOverride.objects.filter(team_id__in=team_ids))
_raw_delete(Person.objects.filter(team_id__in=team_ids))
Expand Down
Loading
Loading