-
Notifications
You must be signed in to change notification settings - Fork 6
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
Migrate to sentry-clj. Getting rid of raven lib. #65
Migrate to sentry-clj. Getting rid of raven lib. #65
Conversation
arthuraliiev
commented
Mar 7, 2024
•
edited
Loading
edited
- Migration to sentry-clj library.
- Getting rid of unsupported raven library.
This pull request has been linked to Shortcut Story #90117: Move K2 apps over to exoscale.sentry.io. |
f6bc78b
to
259aa54
Compare
259aa54
to
cadfcd8
Compare
src/spootnik/reporter/impl.clj
Outdated
;; "sentry" is a sentry map like {:dsn "..."} | ||
;; "raven-options" is the options map sent to raven http client | ||
;; http://aleph.io/codox/aleph/aleph.http.html#var-request | ||
;; "sentry" is a configuration map sent to sentry.io on initialisation |
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.
When migrating to the new reporter, we will have to provide the environment and release, instead of reading it off from SENTRY_*
env vars right? Might be worth it to put it in the readme/changelog
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.
don't get this
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 mean that we may want to update the README to say clients should specifically pass environment and release when sending events.
Eg blockproxy was relying on SENTRY_
env vars: https://github.com/exoscale/blockstorage/blob/master/.kargo/collector.edn#L36 and io.sentry.logback.SentryAppender
, which would read off those env vars; when we move to the new reporter, we have to explicitly pass these variables to the sentry config
5836e1b
to
2311f14
Compare
b977b76
to
fa85dfe
Compare
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.
Did you test it against both the internal sentry & the saas one? We had issues with the golang sdk were they introduced breaking changes preventing the use of the internal one (which was silently failing)
This should break indeed, but it's ok: as part of a reporter bump you'd move to saas sentry. |