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.

Sign up for GitHub

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(feedback): add a trace.id tag during ingestion #81875

Merged
merged 6 commits into from
Jan 6, 2025
+112 −61
Merged

feat(feedback): add a trace.id tag during ingestion #81875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cdb84b6
feat(feedback): add a trace.id tag during ingestion
aliu39 Dec 9, 2024
bfc9411
Restore code in test_fix_for_issue_platform
aliu39 Dec 9, 2024
404d571
Rm mention of alert condition - ids are uuids
aliu39 Dec 9, 2024
5823e11
Move tags logic to create_feedback_issue
aliu39 Dec 9, 2024
f9b44fd
Merge branch 'master' into aliu/feedback-trace-tag
aliu39 Jan 6, 2025
9d8fb7c
Ref user email condition
aliu39 Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions src/sentry/feedback/usecases/create_feedback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,16 @@ def make_evidence(feedback, source: FeedbackCreationSource, is_message_spam: boo
return evidence_data, evidence_display


def fix_for_issue_platform(event_data):
# the issue platform has slightly different requirements than ingest
# for event schema, so we need to massage the data a bit
def fix_for_issue_platform(event_data: dict[str, Any]) -> dict[str, Any]:
"""
The issue platform has slightly different requirements than ingest for event schema,
so we need to massage the data a bit.
* event["tags"] is coerced to a dict.
* If event["user"]["email"] is missing we try to set using the feedback context.

Returns:
A dict[str, Any] conforming to sentry.issues.json_schemas.EVENT_PAYLOAD_SCHEMA.
"""
ret_event: dict[str, Any] = {}

ret_event["timestamp"] = datetime.fromtimestamp(event_data["timestamp"], UTC).isoformat()
Expand Down Expand Up @@ -147,7 +154,6 @@ def fix_for_issue_platform(event_data):

# Force `tags` to be a dict if it's initially a list,
# since we can't guarantee its type here.

tags = event_data.get("tags", {})
tags_dict = {}
if isinstance(tags, list):
Expand All @@ -157,11 +163,6 @@ def fix_for_issue_platform(event_data):
tags_dict = tags
ret_event["tags"] = tags_dict

# Set the user.email tag since we want to be able to display user.email on the feedback UI as a tag
# as well as be able to write alert conditions on it
if not ret_event["tags"].get("user.email"):
ret_event["tags"]["user.email"] = contact_email

# Set the event message to the feedback message.
ret_event["logentry"] = {"message": feedback_obj.get("message")}

Expand Down Expand Up @@ -316,6 +317,17 @@ def create_feedback_issue(event, project_id: int, source: FeedbackCreationSource
}
event_fixed = fix_for_issue_platform(event_data)

# Set the user.email tag since we want to be able to display user.email on the feedback UI as a tag
# as well as be able to write alert conditions on it
user_email = get_path(event_fixed, "user", "email")
if user_email and "user.email" not in event_fixed["tags"]:
event_fixed["tags"]["user.email"] = user_email

# Set the trace.id tag to expose it for the feedback UI.
trace_id = get_path(event_fixed, "contexts", "trace", "trace_id")
if trace_id:
event_fixed["tags"]["trace.id"] = trace_id

# make sure event data is valid for issue platform
validate_issue_platform_event_schema(event_fixed)

Expand Down
143 changes: 91 additions & 52 deletions tests/sentry/feedback/usecases/test_create_feedback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ def test_fix_for_issue_platform():
fixed_event = fix_for_issue_platform(event)
validate_issue_platform_event_schema(fixed_event)
assert fixed_event["contexts"]["replay"]["replay_id"] == "3d621c61593c4ff9b43f8490a78ae18e"
assert fixed_event["tags"]["user.email"] == "[email protected]"
aliu39 marked this conversation as resolved.
Show resolved Hide resolved
assert fixed_event["contexts"]["feedback"] == {
"contact_email": "[email protected]",
"name": "Josh Ferge",
Expand Down Expand Up @@ -661,6 +660,64 @@ def test_create_feedback_spam_detection_project_option_false(
assert found_is_spam is None


@django_db_all
def test_create_feedback_spam_detection_set_status_ignored(
default_project,
monkeypatch,
):
with Feature(
{
"organizations:user-feedback-spam-filter-actions": True,
"organizations:user-feedback-spam-filter-ingest": True,
}
):
event = {
"project_id": default_project.id,
"request": {
"url": "https://sentry.sentry.io/feedback/?statsPeriod=14d",
"headers": {
"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118.0.0.0 Safari/537.36"
},
},
"event_id": "56b08cf7852c42cbb95e4a6998c66ad6",
"timestamp": 1698255009.574,
"received": "2021-10-24T22:23:29.574000+00:00",
"environment": "prod",
"release": "frontend@daf1316f209d961443664cd6eb4231ca154db502",
"user": {
"ip_address": "72.164.175.154",
"email": "[email protected]",
"id": 880461,
"isStaff": False,
"name": "Josh Ferge",
},
"contexts": {
"feedback": {
"contact_email": "[email protected]",
"name": "Josh Ferge",
"message": "This is definitely spam",
"replay_id": "3d621c61593c4ff9b43f8490a78ae18e",
"url": "https://sentry.sentry.io/feedback/?statsPeriod=14d",
},
},
"breadcrumbs": [],
"platform": "javascript",
}

mock_openai = Mock()
mock_openai().chat.completions.create = create_dummy_response

monkeypatch.setattr("sentry.llm.providers.openai.OpenAI", mock_openai)

create_feedback_issue(
event, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
)

group = Group.objects.get()
assert group.status == GroupStatus.IGNORED
assert group.substatus == GroupSubStatus.FOREVER


@django_db_all
def test_create_feedback_adds_associated_event_id(
default_project, mock_produce_occurrence_to_kafka
Expand Down Expand Up @@ -714,61 +771,43 @@ def test_create_feedback_adds_associated_event_id(


@django_db_all
def test_create_feedback_spam_detection_set_status_ignored(
default_project,
monkeypatch,
):
with Feature(
{
"organizations:user-feedback-spam-filter-actions": True,
"organizations:user-feedback-spam-filter-ingest": True,
}
):
event = {
"project_id": default_project.id,
"request": {
"url": "https://sentry.sentry.io/feedback/?statsPeriod=14d",
"headers": {
"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/118.0.0.0 Safari/537.36"
},
},
"event_id": "56b08cf7852c42cbb95e4a6998c66ad6",
"timestamp": 1698255009.574,
"received": "2021-10-24T22:23:29.574000+00:00",
"environment": "prod",
"release": "frontend@daf1316f209d961443664cd6eb4231ca154db502",
"user": {
"ip_address": "72.164.175.154",
"email": "[email protected]",
"id": 880461,
"isStaff": False,
"name": "Josh Ferge",
},
"contexts": {
"feedback": {
"contact_email": "[email protected]",
"name": "Josh Ferge",
"message": "This is definitely spam",
"replay_id": "3d621c61593c4ff9b43f8490a78ae18e",
"url": "https://sentry.sentry.io/feedback/?statsPeriod=14d",
},
},
"breadcrumbs": [],
"platform": "javascript",
}
def test_create_feedback_tags(default_project, mock_produce_occurrence_to_kafka):
"""We want to surface these tags in the UI. We also use user.email for alert conditions."""
event = mock_feedback_event(default_project.id, datetime.now(UTC))
event["user"]["email"] = "[email protected]"
event["contexts"]["feedback"]["contact_email"] = "[email protected]"
event["contexts"]["trace"] = {"trace_id": "abc123"}
create_feedback_issue(event, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE)

mock_openai = Mock()
mock_openai().chat.completions.create = create_dummy_response
assert mock_produce_occurrence_to_kafka.call_count == 1
produced_event = mock_produce_occurrence_to_kafka.call_args.kwargs["event_data"]
tags = produced_event["tags"]
assert tags["user.email"] == "[email protected]"
assert tags["trace.id"] == "abc123"

monkeypatch.setattr("sentry.llm.providers.openai.OpenAI", mock_openai)
# Uses feedback contact_email if user context doesn't have one
del event["user"]["email"]
create_feedback_issue(event, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE)

create_feedback_issue(
event, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE
)
assert mock_produce_occurrence_to_kafka.call_count == 2 # includes last feedback
produced_event = mock_produce_occurrence_to_kafka.call_args.kwargs["event_data"]
tags = produced_event["tags"]
assert tags["user.email"] == "[email protected]"

group = Group.objects.get()
assert group.status == GroupStatus.IGNORED
assert group.substatus == GroupSubStatus.FOREVER

@django_db_all
def test_create_feedback_tags_skips_if_empty(default_project, mock_produce_occurrence_to_kafka):
event = mock_feedback_event(default_project.id, datetime.now(UTC))
event["user"].pop("email", None)
event["contexts"]["feedback"].pop("contact_email", None)
event["contexts"].pop("trace", None)
create_feedback_issue(event, default_project.id, FeedbackCreationSource.NEW_FEEDBACK_ENVELOPE)

assert mock_produce_occurrence_to_kafka.call_count == 1
produced_event = mock_produce_occurrence_to_kafka.call_args.kwargs["event_data"]
tags = produced_event["tags"]
assert "user.email" not in tags
assert "trace.id" not in tags


@django_db_all
Expand Down
Loading