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(feedback): add POST endpoint for user feedback ingest #55735

Merged
merged 42 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
29d64ab
feat(userfeedback): create django model
michellewzhang Sep 5, 2023
687f063
add replays as codeowner
michellewzhang Sep 5, 2023
e1328aa
add endpoints and test files
michellewzhang Sep 5, 2023
4217b01
add user feedback details endpoint
michellewzhang Sep 5, 2023
75117ac
add to user feedback index endpoint
michellewzhang Sep 5, 2023
179e206
standardize user_feedback naming
michellewzhang Sep 6, 2023
d760621
spacing
michellewzhang Sep 6, 2023
4457568
update app label
michellewzhang Sep 6, 2023
433b7f9
update migration
michellewzhang Sep 6, 2023
b4340b9
update dependencies
michellewzhang Sep 6, 2023
9a1adb5
Merge remote-tracking branch 'origin/master' into mz/django-model
michellewzhang Sep 6, 2023
48ebb57
fixe naming
michellewzhang Sep 6, 2023
b0297e6
Merge branch 'mz/django-model' into mz/endpoints
michellewzhang Sep 6, 2023
5b1a134
update naming
michellewzhang Sep 6, 2023
106580f
update fields
michellewzhang Sep 6, 2023
f5ce1af
Merge branch 'mz/django-model' into mz/endpoints
michellewzhang Sep 6, 2023
a94a03a
fix
michellewzhang Sep 6, 2023
a3b2994
update name to feedback
michellewzhang Sep 6, 2023
d6f1bd3
Merge branch 'mz/django-model' into mz/endpoints
michellewzhang Sep 6, 2023
58a968e
post method and test
michellewzhang Sep 7, 2023
15d33ff
update fields to remove trace and error ids
michellewzhang Sep 7, 2023
9846869
update event_id to be feedback_id
michellewzhang Sep 7, 2023
fd35fc8
Merge branch 'mz/django-model' into mz/endpoints
michellewzhang Sep 7, 2023
6cd5aa4
Merge remote-tracking branch 'origin/master' into mz/endpoints
michellewzhang Sep 7, 2023
e864200
fix naming
michellewzhang Sep 7, 2023
62326c7
Merge remote-tracking branch 'origin/master' into mz/endpoints
michellewzhang Sep 7, 2023
56fd5ee
add feedbaack api owner
michellewzhang Sep 7, 2023
ec70eb0
Merge remote-tracking branch 'origin/master' into mz/endpoints
michellewzhang Sep 7, 2023
7be5a7a
add GET for index and test
michellewzhang Sep 7, 2023
e42046e
Merge remote-tracking branch 'origin/master' into mz/endpoints
michellewzhang Sep 8, 2023
ca88806
add comments about reordering in server.py
michellewzhang Sep 8, 2023
33a6486
update naming to feedback_ingest, clean up code
michellewzhang Sep 8, 2023
f6fe29d
address comments and improved test
michellewzhang Sep 8, 2023
79a314e
add tests
michellewzhang Sep 8, 2023
99e19e8
more comprehensive tests
michellewzhang Sep 8, 2023
58308c7
raise keyerror in validation:
michellewzhang Sep 8, 2023
a9fb59b
update test
michellewzhang Sep 8, 2023
55f1700
make assertions more explicit
michellewzhang Sep 8, 2023
3e4ef78
Merge remote-tracking branch 'origin/master' into mz/endpoints
michellewzhang Sep 11, 2023
e267539
modify endpoint and tests based on blueprint
michellewzhang Sep 11, 2023
37c8c36
dist is optional
michellewzhang Sep 11, 2023
d6053e8
modify test
michellewzhang Sep 11, 2023
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
1 change: 1 addition & 0 deletions src/sentry/api/api_owners.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ class ApiOwner(Enum):
OWNERS_INGEST = "owners-ingest"
OWNERS_NATIVE = "owners-native"
REPLAY = "replay-backend"
FEEDBACK = "feedback-backend"
UNOWNED = "unowned"
7 changes: 7 additions & 0 deletions src/sentry/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
DiscoverSavedQueryDetailEndpoint,
DiscoverSavedQueryVisitEndpoint,
)
from sentry.feedback.endpoints.feedback_ingest import FeedbackIngestEndpoint
from sentry.incidents.endpoints.organization_alert_rule_available_action_index import (
OrganizationAlertRuleAvailableActionIndexEndpoint,
)
Expand Down Expand Up @@ -2915,6 +2916,12 @@
SetupWizard.as_view(),
name="sentry-api-0-project-wizard",
),
# Feedback
re_path(
r"^feedback/$",
FeedbackIngestEndpoint.as_view(),
name="sentry-api-0-feedback-ingest",
),
# Internal
re_path(
r"^internal/",
Expand Down
2 changes: 2 additions & 0 deletions src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ def env(
"REGION": ["sentry.RegionOutbox"],
}

# Do not modify reordering
# The applications listed first in INSTALLED_APPS have precedence
INSTALLED_APPS: tuple[str, ...] = (
"django.contrib.auth",
"django.contrib.contenttypes",
Expand Down
Empty file.
153 changes: 153 additions & 0 deletions src/sentry/feedback/endpoints/feedback_ingest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
from __future__ import annotations

import datetime
from typing import Any, Dict
from uuid import uuid4

from rest_framework import serializers
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import features
from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.authentication import (
ApiKeyAuthentication,
DSNAuthentication,
OrgAuthTokenAuthentication,
TokenAuthentication,
)
from sentry.api.base import Endpoint, region_silo_endpoint
from sentry.api.bases.project import ProjectPermission
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.constants import ObjectStatus
from sentry.feedback.models import Feedback
from sentry.models import Organization, ProjectKey
from sentry.models.project import Project
from sentry.utils.sdk import bind_organization_context, configure_scope


class FeedbackValidator(serializers.Serializer):
# required fields
environment = serializers.CharField(required=True)
feedback = serializers.JSONField(required=True)
platform = serializers.CharField(required=True)
release = serializers.CharField(required=True)
sdk = serializers.JSONField(required=True)
timestamp = serializers.FloatField(required=True)

# optional fields
dist = serializers.CharField(required=False)
event_id = serializers.CharField(required=False)
request = serializers.JSONField(required=False)
tags = serializers.JSONField(required=False)
user = serializers.JSONField(required=False)

def validate(self, data):
try:
ret: Dict[str, Any] = {}
ret["data"] = {
"environment": data["environment"],
"feedback": data["feedback"],
"platform": data["platform"],
"release": data["release"],
"sdk": data["sdk"],
"request": data.get("request"),
"user": data.get("user"),
"tags": data.get("tags"),
"dist": data.get("dist"),
}
ret["date_added"] = datetime.datetime.fromtimestamp(data["timestamp"])
ret["feedback_id"] = data.get("event_id") or uuid4().hex
ret["url"] = data["feedback"]["url"]
ret["message"] = data["feedback"]["message"]
ret["replay_id"] = data["feedback"].get("replay_id")
ret["project_id"] = self.context["project"].id

return ret
except KeyError:
raise serializers.ValidationError("Input has wrong field name or type")


class FeedbackIngestPermission(ProjectPermission):
scope_map = {
"POST": ["project:read", "project:write", "project:admin"],
}


@region_silo_endpoint
class FeedbackIngestEndpoint(Endpoint):
publish_status = {
"POST": ApiPublishStatus.EXPERIMENTAL,
}
owner = ApiOwner.FEEDBACK

# Authentication code borrowed from the monitor endpoints (which will eventually be removed)
authentication_classes = (
DSNAuthentication,
TokenAuthentication,
OrgAuthTokenAuthentication,
ApiKeyAuthentication,
)

permission_classes = (FeedbackIngestPermission,)

def convert_args(
JoshFerge marked this conversation as resolved.
Show resolved Hide resolved
self,
request: Request,
organization_slug: str | None = None,
*args,
**kwargs,
):
using_dsn_auth = isinstance(request.auth, ProjectKey)

# When using DSN auth we're able to infer the organization slug
if not organization_slug and using_dsn_auth:
organization_slug = request.auth.project.organization.slug # type: ignore

if organization_slug:
try:
organization = Organization.objects.get_from_cache(slug=organization_slug)
# Try lookup by slug first. This requires organization context since
# slugs are unique only to the organization
except (Organization.DoesNotExist):
raise ResourceDoesNotExist

project = request.auth.project # type: ignore

if project.status != ObjectStatus.ACTIVE:
raise ResourceDoesNotExist

if using_dsn_auth and project.id != request.auth.project_id: # type: ignore
raise ResourceDoesNotExist

if organization_slug and project.organization.slug != organization_slug:
raise ResourceDoesNotExist

# Check project permission. Required for Token style authentication
self.check_object_permissions(request, project)

with configure_scope() as scope:
scope.set_tag("project", project.id)

bind_organization_context(project.organization)

request._request.organization = project.organization # type: ignore

kwargs["organization"] = organization
kwargs["project"] = project
return args, kwargs

def post(self, request: Request, organization: Organization, project: Project) -> Response:
if not features.has(
"organizations:user-feedback-ingest", project.organization, actor=request.user
):
return Response(status=404)

feedback_validator = FeedbackValidator(data=request.data, context={"project": project})
michellewzhang marked this conversation as resolved.
Show resolved Hide resolved
if not feedback_validator.is_valid():
return self.respond(feedback_validator.errors, status=400)

result = feedback_validator.validated_data
Feedback.objects.create(**result)
return self.respond(status=201)
194 changes: 194 additions & 0 deletions tests/sentry/feedback/test_feedback_ingest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
from django.urls import reverse
from rest_framework.exceptions import ErrorDetail

from sentry.feedback.models import Feedback
from sentry.testutils.cases import MonitorIngestTestCase

test_data = {
michellewzhang marked this conversation as resolved.
Show resolved Hide resolved
"dist": "abc123",
"environment": "production",
"event_id": "1ffe0775ac0f4417aed9de36d9f6f8dc",
"feedback": {
"contact_email": "colton.allen@sentry.io",
"message": "I really like this user-feedback feature!",
"replay_id": "ec3b4dc8b79f417596f7a1aa4fcca5d2",
"url": "https://docs.sentry.io/platforms/javascript/",
},
"platform": "javascript",
"release": "version@1.3",
"request": {
"headers": {
"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36"
}
},
"sdk": {"name": "sentry.javascript.react", "version": "6.18.1"},
"tags": {"key": "value"},
"timestamp": 1234456,
"user": {
"email": "username@example.com",
"id": "123",
"ip_address": "127.0.0.1",
"name": "user",
"username": "user2270129",
},
}


class FeedbackIngestTest(MonitorIngestTestCase):
endpoint = "sentry-api-0-feedback-ingest"

def test_save_feedback(self):
# Feature enabled should lead to successful save
with self.feature({"organizations:user-feedback-ingest": True}):
path = reverse(self.endpoint)
response = self.client.post(path, data=test_data, **self.dsn_auth_headers)
assert response.status_code == 201

# Feedback object exists
feedback_list = Feedback.objects.all()
assert len(feedback_list) == 1

michellewzhang marked this conversation as resolved.
Show resolved Hide resolved
# Feedback object is what was posted
feedback = feedback_list[0]
assert feedback.data["dist"] == "abc123"
assert feedback.data["environment"] == "production"
assert feedback.data["sdk"]["name"] == "sentry.javascript.react"
assert feedback.data["feedback"]["contact_email"] == "colton.allen@sentry.io"
assert (
feedback.data["feedback"]["message"] == "I really like this user-feedback feature!"
)
assert feedback.data["tags"]["key"] == "value"
assert feedback.data["release"] == "version@1.3"
assert feedback.data["user"]["name"] == "user"
assert (
feedback.data["request"]["headers"]["User-Agent"]
== "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36"
)
assert feedback.data["platform"] == "javascript"

def test_no_feature_enabled(self):
# Feature disabled should lead to unsuccessful save
with self.feature({"organizations:user-feedback-ingest": False}):
path = reverse(self.endpoint)
response = self.client.post(path, data=test_data, **self.dsn_auth_headers)
assert response.status_code == 404

def test_not_authorized(self):
# No authorization should lead to unsuccessful save
with self.feature({"organizations:user-feedback-ingest": True}):
path = reverse(self.endpoint)
response = self.client.post(path, data=test_data)
assert response.status_code == 401
assert response.data == {"detail": "Authentication credentials were not provided."}
michellewzhang marked this conversation as resolved.
Show resolved Hide resolved

def test_wrong_input(self):
# Wrong inputs should lead to failed validation
wrong_test_data = {
"dist!": "abc",
"environment": "production",
"feedback": {
"contact_email": "colton.allen@sentry.io",
"message": "I really like this user-feedback feature!",
"replay_id": "ec3b4dc8b79f417596f7a1aa4fcca5d2",
"url123": "https://docs.sentry.io/platforms/javascript/",
},
"platform": "javascript",
"release": "version@1.3",
"sdk": {"name": "sentry.javascript.react", "version": "6.18.1"},
"timestamp": 1234456,
}

with self.feature({"organizations:user-feedback-ingest": True}):
path = reverse(self.endpoint)
response = self.client.post(path, data=wrong_test_data, **self.dsn_auth_headers)
assert response.status_code == 400
assert response.data == {
"non_field_errors": [
ErrorDetail(string="Input has wrong field name or type", code="invalid")
]
}

def test_no_environment(self):
# Environment field is required for a successful post
missing_environment_test_data = {
"dist": "abc123",
"feedback": {
"contact_email": "colton.allen@sentry.io",
"message": "I really like this user-feedback feature!",
"replay_id": "ec3b4dc8b79f417596f7a1aa4fcca5d2",
"url": "https://docs.sentry.io/platforms/javascript/",
},
"platform": "javascript",
"release": "version@1.3",
"request": {
"headers": {
"User-Agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/103.0.0.0 Safari/537.36"
}
},
"sdk": {"name": "sentry.javascript.react", "version": "6.18.1"},
"tags": {"key": "value"},
"timestamp": 1234456,
}

with self.feature({"organizations:user-feedback-ingest": True}):
path = reverse(self.endpoint)
response = self.client.post(
path, data=missing_environment_test_data, **self.dsn_auth_headers
)
assert response.status_code == 400
assert response.data == {
"environment": [ErrorDetail(string="This field is required.", code="required")]
}

def test_wrong_type(self):
# Fields should be correct type
wrong_type_test_data = {
"feedback": {
"contact_email": "colton.allen@sentry.io",
"message": "I really like this user-feedback feature!",
"replay_id": "ec3b4dc8b79f417596f7a1aa4fcca5d2",
"url": "https://docs.sentry.io/platforms/javascript/",
},
"environment": {},
"platform": "javascript",
"release": "1",
"sdk": {"name": "sentry.javascript.react", "version": "6.18.1"},
"timestamp": 123456,
}

with self.feature({"organizations:user-feedback-ingest": True}):
path = reverse(self.endpoint)
response = self.client.post(path, data=wrong_type_test_data, **self.dsn_auth_headers)
assert response.status_code == 400
michellewzhang marked this conversation as resolved.
Show resolved Hide resolved
assert response.data == {
"environment": [ErrorDetail(string="Not a valid string.", code="invalid")]
}

def test_bad_slug_path(self):
# Bad slug in path should lead to unsuccessful save
with self.feature({"organizations:user-feedback-ingest": True}):
path = reverse(self.endpoint)
response = self.client.post(path + "bad_slug", data=test_data, **self.dsn_auth_headers)
assert response.status_code == 404

def test_missing_optional_fields(self):
# Optional fields missing should still result in successful save
test_data_missing_optional_fields = {
"environment": "production",
"feedback": {
"contact_email": "colton.allen@sentry.io",
"message": "I really like this user-feedback feature!",
"url": "https://docs.sentry.io/platforms/javascript/",
},
"platform": "javascript",
"release": "version@1.3",
"sdk": {"name": "sentry.javascript.react", "version": "6.18.1"},
"timestamp": 1234456,
}
michellewzhang marked this conversation as resolved.
Show resolved Hide resolved

with self.feature({"organizations:user-feedback-ingest": True}):
path = reverse(self.endpoint)
response = self.client.post(
path, data=test_data_missing_optional_fields, **self.dsn_auth_headers
)
assert response.status_code == 201
Loading