-
-
Notifications
You must be signed in to change notification settings - Fork 1
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: Add basic schema and examples for ingest-events [INC-660] #230
Changes from all commits
7467953
6bb7c1b
3959cd7
46ecf30
7c191b3
4d97466
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 |
---|---|---|
@@ -0,0 +1 @@ | ||
‡¤type¥event§payloadÅŽ{"event_id":"2107c4e968514c8690d3a3a24570c40d","level":"info","version":"7","type":"default","transaction_info":{},"logentry":{"formatted":"Something went wrong"},"logger":"","modules":{"certifi":"2024.2.2","pip":"23.3.1","pywatchman":"1.4.1","sentry-sdk":"1.40.6","setuptools":"68.2.2","urllib3":"2.2.1","wheel":"0.41.3"},"platform":"python","timestamp":1709691573.27258,"received":1709691573.415304,"environment":"production","contexts":{"runtime":{"name":"CPython","version":"3.11.6","build":"3.11.6 (main, Nov 2 2023, 04:39:43) [Clang 14.0.3 (clang-1403.0.22.14.1)]","type":"runtime"},"trace":{"trace_id":"97791ab406224f03be50f19307648648","span_id":"864321f6ade8a8d9","status":"unknown","type":"trace"}},"tags":[["server_name","CH26FF6WL3"]],"extra":{"sys.argv":["send_event.py"]},"sdk":{"name":"sentry.python","version":"1.40.6","integrations":["argv","atexit","dedupe","excepthook","logging","modules","stdlib","threading"],"packages":[{"name":"pypi:sentry-sdk","version":"1.40.6"}]},"key_id":"3","project":3,"grouping_config":{"enhancements":"KLUv_SAYwQAAkwKRs25ld3N0eWxlOjIwMjMtMDEtMTGQ","id":"newstyle:2023-01-11"},"_metrics":{"bytes.ingested.event":906}}ªstart_timeÎeçÒµ¨event_idÙ 2107c4e968514c8690d3a3a24570c40dªproject_id«remote_addrª172.18.0.1«attachments |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
{ | ||
"$schema": "http://json-schema.org/draft-07/schema#", | ||
"description": "Error message from Relay", | ||
"type": "object", | ||
"properties": { | ||
"type": { "type": "string" }, | ||
"payload": { | ||
"description": "bytes" | ||
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. Is 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. only for humans, this is not doing any validation |
||
}, | ||
Comment on lines
+7
to
+9
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. JSON schema allows additionalProperties by default, right? Because Relay does send more fields: https://github.com/getsentry/relay/blob/87b23e6953cd18a0d591c9d13e43baee0d0d4b95/relay-server/src/services/store.rs#L1268-L1271 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. It does -- the example included here also has those extra fields and is passing the tests |
||
"event_id": { "type": "string" }, | ||
"project_id": { "type": "integer" }, | ||
"start_time": { "type": "integer" } | ||
}, | ||
"required": ["type", "event_id", "project_id", "payload", "start_time"] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
topic: ingest-events | ||
pipeline: errors | ||
description: Errors data from Relay | ||
services: | ||
producers: | ||
- getsentry/relay | ||
consumers: | ||
- getsentry/sentry | ||
schemas: | ||
- version: 1 | ||
compatibility_mode: none | ||
type: msgpack | ||
resource: ingest-events.v1.schema.json | ||
examples: | ||
- ingest-events/1/ | ||
topic_creation_config: | ||
max.message.bytes: "10000000" |
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.
To double check: are there other message types sharing this topic right now? User feedback?
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.
Yes! Until we have migrated relay, SDK, etc to ingest-feedback-events
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.
I compared this to the example you added in #227.
event_id and type is there, but you have no project_id or payload in your schema? Is that actually true? How do you know what project the user feedback belongs to?
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.
I based my fields on this https://develop.sentry.dev/sdk/event-payloads/#required-attributes
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.
There is probably project_id in every event, and I can add that. But don't want to make it required, since I don't think it's required for feedback events. (In fact it turns out the feedback fields themselves are all optional 😅)
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.
The link you sent represents the payload that the user sends to Relay though, not the one received on this topic which happens after Relay processing. I assume the project ID is automatically added by Relay and is not optional, as a user feedback that doesn't belong to any project doesn't really make any sense.
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.
Looking at https://github.com/getsentry/sentry/blob/a1c47440d4399f571fe0161b441811d48148e43c/src/sentry/ingest/consumer/simple_event.py#L42 it seems like a message without a project_id would crash the consumer