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.

Sign up for GitHub

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

Jump to bottom

Add Environment and Release option to logging-sentry #7833

Merged
merged 2 commits into from
Mar 13, 2020
Merged

Add Environment and Release option to logging-sentry #7833

merged 2 commits into from
Mar 13, 2020

Conversation

MarcBoissonneault
Copy link
Contributor

@MarcBoissonneault MarcBoissonneault commented Mar 13, 2020 •
edited
Loading

Details:

  • Add config fields for Environment and Release in SentryConfig.java
  • Add provider functionality to return configured Environment and Release in SentryConfigProvider
  • Add Tests to prove the configs are loaded into Sentry Client.

GitHub Issue: #7821

@MarcBoissonneault MarcBoissonneault marked this pull request as ready for review March 13, 2020 01:47
Add Environment & Release option to logging-sentry
9a101fa 
Details:
- Add config fields for Environment and Release in `SentryConfig.java`
- Add provider functionality to return configured Environment and Release in `SentryConfigProvider`
- Add Tests to prove the configs are loaded into Sentry Client.

GitHub Issue: #7821
@gsmet
Copy link
Member

gsmet commented Mar 13, 2020

AFAICS, the added tests don't pass:

[ERROR]   SentryLoggerEnvironmentOptionTests.sentryLoggerEnvironmentOptionTest:27 
Expecting:
 <"">
to be equal to:
 <"test-environment">
but was not.
[ERROR]   SentryLoggerReleaseOptionTests.sentryLoggerEnvironmentOptionTest:27 
Expecting:
 <"">
to be equal to:
 <"releaseABC">
but was not.

@gsmet
Copy link
Member

gsmet commented Mar 13, 2020

@MarcBoissonneault thanks for your contribution. I don't see any immediate reason why the tests fail, could you have a look?

@ia3andy I let you help @MarcBoissonneault on this one?

@ia3andy ia3andy self-assigned this Mar 13, 2020
@ia3andy
Copy link
Contributor

ia3andy commented Mar 13, 2020

@MarcBoissonneault did the tests pass locally? I will check it out and debug.. the code looks good.

ia3andy
ia3andy requested changes Mar 13, 2020
View reviewed changes
ia3andy
ia3andy approved these changes Mar 13, 2020
View reviewed changes
Copy link
Contributor

@ia3andy ia3andy left a comment

Choose a reason for hiding this comment

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

Thanks for the contrib!!

@ia3andy ia3andy merged commit 650ed3d into quarkusio:master Mar 13, 2020
@MarcBoissonneault
Copy link
Contributor Author

@gsmet Sorry I was sleeping 3 hours ago (we are surely not in the same time-zone!)
@ia3andy Thanks for fixing the inverted configs (and all the text issues)!

@MarcBoissonneault MarcBoissonneault mentioned this pull request Mar 17, 2020
Closed
@gsmet gsmet changed the title Add Environment & Release option to logging-sentry Add Environment and Release option to logging-sentry Mar 21, 2020
@gsmet gsmet added this to the 1.4.0 milestone Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ia3andy ia3andy ia3andy approved these changes

Assignees

@ia3andy ia3andy

Labels
area/logging
Projects
None yet
Milestone
1.4.0.CR1
Development

Successfully merging this pull request may close these issues.
3 participants
@MarcBoissonneault @gsmet @ia3andy