Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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): add kafka config for new ingest-feedbacks consumer #66169
feat(replay): add kafka config for new ingest-feedbacks consumer #66169
Changes from 5 commits
62dd685
78cedd6
2552f1a
7292129
f0f614a
4dc60ad
69e2a34
9d26087
35a45b3
9ae6f76
1b3e8c4
8763274
0f346d9
8823af4
99e584e
81f6adc
83e2e1c
b1e0f4e
05b4d67
cb4ca9d
ddbc653
9ab3434
dc7d11c
23a731e
edd1f49
4a5aa18
ed56fba
9750bb4
d78df18
06b99fb
64bbcdf
7be67a1
b611054
d367e7b
7f1dd10
5de51dc
32b35e7
ce126ce
302e39b
f79cdb0
c94ba94
608780f
8ed8f67
1cebf7f
b01c76a
a39c154
72cf7e3
2906d16
64ba793
65c709f
625ae35
29cd673
fcadc17
3ca6676
1794c57
a0359e9
e18b1be
89bf6ae
c6fa668
274acff
1b4fba0
468f10f
242e6a6
0029dd8
bf65b9e
ff74225
e574575
db28b55
5a5b331
9e6f13b
f8649f6
442f7e4
8631dad
b9d94b8
1a3dbf6
7445433
9e85a14
7d16cb4
5005304
ef8ec57
57a37c3
75f021c
8fd0983
dde199c
1525c6b
7ff66f2
f143d17
83faf43
770de43
e8cedc2
e51094f
910ed95
58b07d2
0c285c0
3afdb48
0ca6661
2a749cd
01103b7
ce143c1
1445beb
de9eedd
2b9958a
cfd8949
b0a2c9e
345f8d9
e3330ff
6e2a7b3
739564a
5007096
703029c
5c9084c
8485db2
90b2ace
ff5e9d2
8bf591b
a5e235c
300a604
c7e9a43
fb5a7a7
64a74e2
3d24962
8b74892
d286f50
fd11248
06ed897
c7f8945
efb1f92
99de5a5
b1d5691
2bbcdc7
5e372c0
fd1151d
a23749e
bd775f9
3bea14c
1537c1f
cdaf91c
5c7f1f2
dfd60b1
a79af6e
36afa8a
de7fb8c
a63025a
55b3275
c0ede95
b49b81a
47f0b8f
55fede8
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Each consumer should have it's own strategy. The ingest strategy is already doing far more than it should and we should not add additional logic to it.
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.
Any tips/references for how to write a new strategy? In ingest/consumer/factory.py, right
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 directory structure in sentry is a bit misleading, but the
ingest
module really just refers to the errors/transactions ingest consumer today and not any of the rest of the piplines. I would suggest keeping it either entirely separate, or inside a new moduleingest.user_feedback
or something like that. See https://getsentry.github.io/arroyo/ for docs how to write a strategy.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.
@lynnagara We're not adding additional logic to it. This behavior already exists in that consumer. We're just moving it to a new topic and new consumer infrastructure. I agree we should split out this product into its own consumer. Can that be handled in a follow-up PR?
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.
Ah makes sense, I thought it was totally new functionality.
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 think you might want to make this a new type. We don't really want to run all the
events
logic and configuration (even if they share the bulk of code in the initial implementation), and i'm guessing this will diverge in the future?