-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
refactor sentry config #7757
refactor sentry config #7757
Conversation
I see that you haven't updated any CHANGELOG files. Would it make sense to do so? |
@@ -5,4 +5,4 @@ cd "$(dirname "$0")" | |||
COMMIT_SHA=${COMMIT_SHA:-$(git rev-parse HEAD)} | |||
VERSION=${VERSION:-$(cat "../../VERSION")} | |||
|
|||
echo "-X github.com/smartcontractkit/chainlink/core/static.Version=$VERSION -X github.com/smartcontractkit/chainlink/core/static.Sha=$COMMIT_SHA -X github.com/smartcontractkit/chainlink/core/static.SentryDSN=$SENTRY_DSN" |
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.
static.SentryDSN
actually doesn't exist 🤷
50b2728
to
e584321
Compare
sentryrelease = static.Version | ||
} | ||
|
||
return sentry.Init(sentry.ClientOptions{ |
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.
Uff, initialising global variable from application instance. In theory we could have multiple application instances, can you think of any way around 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.
In theory yes, but in implementation no, because we use an EmptyRunner
in tests. Also, empirically, the gin stuff would have the same problem, but has been here over a year without flagging any races.
https://app.shortcut.com/chainlinklabs/story/23679/prefix-all-env-vars-with-cl
Refactor sentry init to read from config instead of direct env vars.