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

Resolve HTTP Proxy parameters from the external configuration. #1035

Merged
merged 4 commits into from
Nov 16, 2020

Conversation

maciejwalkowiak
Copy link
Contributor

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Resolve HTTP Proxy parameters from the external configuration.

In order to make Proxy properties resolution in non-Spring Boot application the same as in Spring Boot application, the names of the environment variables had to be changed from ones in 1.7.x

  • SENTRY_PROXY_HOST
  • SENTRY_PROXY_PORT
  • SENTRY_PROXY_USER
  • SENTRY_PROXY_PASS

In addition to that - SentryOptions#proxy object had to be changed into Java bean so that we are able to bind properties via Spring Boot environment.

💡 Motivation and Context

Fixes #1028

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing
  • No breaking changes

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

codecov-io commented Nov 10, 2020

Codecov Report

Merging #1035 (eb3276a) into main (296cc93) will decrease coverage by 0.00%.
The diff coverage is 73.52%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1035      +/-   ##
============================================
- Coverage     71.98%   71.98%   -0.01%     
- Complexity     1322     1336      +14     
============================================
  Files           135      138       +3     
  Lines          4816     4880      +64     
  Branches        492      499       +7     
============================================
+ Hits           3467     3513      +46     
- Misses         1091     1106      +15     
- Partials        258      261       +3     
Impacted Files Coverage Δ Complexity Δ
...java/io/sentry/transport/AuthenticatorWrapper.java 50.00% <50.00%> (ø) 2.00 <2.00> (?)
sentry/src/main/java/io/sentry/SentryOptions.java 79.82% <66.66%> (-1.95%) 92.00 <0.00> (+2.00) ⬇️
...n/java/io/sentry/transport/ProxyAuthenticator.java 66.66% <66.66%> (ø) 3.00 <3.00> (?)
...c/main/java/io/sentry/transport/HttpTransport.java 81.35% <86.95%> (+0.34%) 40.00 <3.00> (+5.00)
...main/java/io/sentry/config/PropertiesProvider.java 100.00% <100.00%> (ø) 2.00 <2.00> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 296cc93...eb3276a. Read the comment docs.

@marandaneto
Copy link
Contributor

@maciejwalkowiak this is ready to be merged, right? let's do it so we include it in the release next week.

@marandaneto marandaneto merged commit 94691a2 into main Nov 16, 2020
@marandaneto marandaneto deleted the gh-1028 branch November 16, 2020 12:09
maciejwalkowiak added a commit to maciejwalkowiak/sentry-docs that referenced this pull request Nov 23, 2020
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.

version 3.1.2 fails to use SENTRY_HTTP_PROXY_* variables
3 participants