Skip to content
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

sentry: allow specify a DSN for backend #17363

Merged
merged 2 commits into from
Jan 18, 2021
Merged

sentry: allow specify a DSN for backend #17363

merged 2 commits into from
Jan 18, 2021

Conversation

unknwon
Copy link
Member

@unknwon unknwon commented Jan 18, 2021

Adds a new site config option "log": { "sentry": { "backendDSN": "<REDACTED>" } } to set a separate Sentry DSN only for backend errors.

Part of #16109

@unknwon unknwon marked this pull request as ready for review January 18, 2021 11:57
@unknwon unknwon requested a review from a team January 18, 2021 11:57
@@ -60,12 +60,21 @@ func Init() {
return
}

if conf.Get().Log.Sentry == nil {
sentryConfig := conf.Get().Log.Sentry
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can Log ever be nil?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, it's a pointer so it can; practically, unlikely (not sure though).

Copy link
Member Author

@unknwon unknwon Jan 18, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I misunderstood your comment.

5 lines above, we checked Log:

if conf.Get().Log == nil {


// Fallback to default DSN if the backend DSN is not specified separately
if backendDSN == "" {
backendDSN = sentryConfig.Dsn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: BackendDSN and Dsn have different cases fordsn

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh... you're right, but Dsn is code-generated (which I hope one day it could generate DSN, HTTP etc.). For native Go names, I prefer DSN over Dsn.

@codecov
Copy link

codecov bot commented Jan 18, 2021

Codecov Report

Merging #17363 (432008e) into main (c0c43b7) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main   #17363      +/-   ##
==========================================
- Coverage   51.88%   51.88%   -0.01%     
==========================================
  Files        1713     1713              
  Lines       84963    84963              
  Branches     7588     7748     +160     
==========================================
- Hits        44083    44080       -3     
- Misses      36976    36977       +1     
- Partials     3904     3906       +2     
Flag Coverage Δ
go 50.83% <ø> (-0.01%) ⬇️
integration 30.72% <ø> (+<0.01%) ⬆️
storybook 30.40% <ø> (ø)
typescript 54.40% <ø> (-0.01%) ⬇️
unit 34.76% <ø> (ø)
Impacted Files Coverage Δ
client/web/src/tree/TreeRoot.tsx 83.33% <0.00%> (-3.04%) ⬇️
.../internal/codeintel/resolvers/graphql/locations.go 83.50% <0.00%> (-2.07%) ⬇️
...nal/campaigns/resolvers/changeset_apply_preview.go 65.92% <0.00%> (+0.74%) ⬆️

@unknwon unknwon merged commit 798371e into main Jan 18, 2021
@unknwon unknwon deleted the cj/sentry-backend-dsn branch January 18, 2021 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants