-
-
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(eap): Add EAP mutations topic #333
Conversation
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.
other than 1 comment, lgtm
CODEOWNERS
Outdated
@@ -1,126 +1,142 @@ | |||
# Search and storage and infra is the default owner | |||
* @getsentry/owners-snuba @getsentry/ops |
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.
this entire file got reformatted, please revert that
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.
Not sure why but every time I try to save it, it reverts to reformatting it. Do you know how to fix that?
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.
turn off format-on-save in your editor for this file
@ayirr7 if it's really the bot reformatting the file, forget about it, just merge then. sorry for the noise |
"description": "The trace ID is a unique identifier for a trace. It is a 16 byte hexadecimal string." | ||
}, | ||
"span_id": { | ||
"type": "integer" |
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.
@untitaker Had some questions here -- I was comparing against the spans topic schema, and I think span_id
is a String? I noticed a couple of other examples like this too
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.
you are right. please change this in the schema. it should not be an int at all, that's a mistake in the code
Doing this separately so we can roll it out independently of the actual consumer: #6216 This will be rolled out after the schema is published getsentry/sentry-kafka-schemas#333
Adds the schema, topic metadata, and an example for the new Snuba eap mutations topic
Relevant to getsentry/snuba#6216