You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR allows sentry-ruby to use proxy config passed via environment variables, i.e. HTTP_PROXY. Closes#2131. The actual configuration settings is still respected over environment variables.
Changes
As @sl0thentr0py suggests, just removes the nil passed into ::Net::HTTP, and allows it to do all the work for us.
I'll add a separate PR into sentry-docs to document how proxy config works. As of right now, transport_configuration block there is minimal.
Once this is shipped, further update sentry-docs with proxy via env var support.
Review
@stephanie-anderson, with this PR, config.transport_configuration.proxy is preferred over the environment values:
If the config.transport_configuration.proxy is present, but malformed (does not have the minimally required uri key), it will still be used over HTTP_PROXY, even if that has the correct setup.
We should probably be consistent with how our other SDKs work with env vars — should we generally prefer the config passed in code, or the env vars?
Merging #2161 (a706d3f) into master (7a65d3f) will increase coverage by 0.00%.
Report is 14 commits behind head on master.
The diff coverage is 100.00%.
I'm still on this — proved to be difficult to get to locally (2.4 doesn't easily install on M1 with rbenv, so I'll docker this tomorrow). I have no clue why this works with Ruby 2.4 on Rack 2, but not on Ruby 2.4 and Rack 3 =/
The CI fails because Ruby didn't support getting user/pass from http_proxy until 2.5 with this commit. I think you need to add Ruby version condition on both proxy_user and proxy_pass assertions.
Since this is on the client SDK side, and it already shipped in 5.14, just update your sentry-ruby, it should already work against both SaaS, and a self-hosted Sentry.
This patch only controls whether the client SDK will use a proxy to send data to the server. Point it to a self-hosted DSN and it should work.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR allows
sentry-rubyto use proxy config passed via environment variables, i.e.HTTP_PROXY. Closes #2131. The actual configuration settings is still respected over environment variables.Changes
nilpassed into::Net::HTTP, and allows it to do all the work for us.TODOs
sentry-ruby/sentry-ruby/spec/sentry/transport/http_transport_spec.rb
Line 94 in c9b4eac
sentry-docsto document how proxy config works. As of right now,transport_configurationblock there is minimal.sentry-docswith proxy via env var support.Review
config.transport_configuration.proxyis preferred over the environment values:config.transport_configuration.proxyis present, but malformed (does not have the minimally required uri key), it will still be used overHTTP_PROXY, even if that has the correct setup.