-
-
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(replay): Config new ingest-feedback-events topic #227
Conversation
topics/ingest-feedbacks.yaml
Outdated
@@ -0,0 +1,17 @@ | |||
topic: ingest-feedbacks | |||
pipeline: replays |
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.
is it actually part of the replays product? isn't this just general user feedback which may or may not be associated with a replay?
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 it's not always associated. can you elaborate on what "pipeline" is for?
The idea was to use the same cluster/infrastructure, not cause the products are dependent, but because the same team owns them. Also to save costs
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.
it provides an view on what features are linked together and how data flows through the system.
As an example, if ingest-events
is broken, then data is not going through to events
and other related topics as well as they feed into each other.
It's got nothing to do with which teams are working on what at sentry at any given point it time (which unfortunately seems to change every quarter).
I think you could probably just make this errors
here, since it's moving from the shared errors topic today.
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.
Ok, thanks for the explanation! what does the errors
pipeline's dependency chain look like?
I think part of the reason we want to move it off that topic is because feedback is not dependent -- see motivation in getsentry/sentry#66100. Thoughts on making a new pipeline for this topic?
Also, can we still have it on the same cluster, if it's a different pipeline? And does cluster need to be defined in some config file?
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's quite a few things in the errors pipeline (you can search for the files in this director that contain errors
if you want to see some example topics). New pipeline sounds fine -- you can just change it here to user-feedback
or whatever you like here, there's nothing else to do.
Yes, the cluster topology is independent and can always be mapped to whatever kafka cluster you like per environment later.
topics/ingest-feedbacks.yaml
Outdated
topic_creation_config: | ||
message.timestamp.type: LogAppendTime |
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 don't think you need this, you can remove the whole topic_creation_config block unless you have some special configuration you want to enforce
1399c42
to
317c598
Compare
Summary of last commit:
Open to discussion on the renaming! Not sure if @bmckerry has already provisioned ingest-feedbacks, and if so, is it rename-able? |
https://getsentry.atlassian.net/browse/OPS-5317 getsentry/sentry#66100 Pipeline/cluster = replays, since replay team owns user feedback. We expect the volume to be too low to justify its own cluster.
+small change to descrip
- move to new 'user-feedback' pipeline - add an initial schema generated from a widget feedback (captured by chrome dev tools) and https://github.com/glideapps/quicktype - rename to 'ingest-feedback-events' since 'FeedbackEvent' seems to be the corresponding type in Django + SDK/typescript
…_email, message, name, source}} - all fields have additionalProperties = true, making schema totally flexible and backward/forward-compatible - rename examples folder
ecd0107
to
6c579a7
Compare
@@ -0,0 +1,67 @@ | |||
{ |
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.
How come these examples don't have project_id? Isn't that a required field in user feedback? It's not on the schema either.
- turns out relay wraps the previous schema in the msgpack 'payload' field, as an encoded JSON string - new schema is almost identical to ingest-events. Bytes is a not a valid jsonschema type so we omit type from the payload
Turns out relay wraps my previous schema in the msgpack 'payload' field, as an encoded JSON string. So the new schema is identical to ingest-events. Bytes is a not a valid jsonschema type so we have to omit type from 'payload' |
https://getsentry.atlassian.net/browse/OPS-5317
getsentry/sentry#66100
New kafka topic for user feedback events, sourced from feedback widget or crash report modal. Schema is identical to ingest-events since feedback is currently in this topic.
Schema references (structure of the message payload):
---> logging in this fx was used to create one of the 2 examples
Also see getsentry/sentry#66484