getsentry/sentry-python

Add `no_proxy` config option in SDK init

Closed this issue · 5 comments

scarletnguyen13 commented

Problem Statement

The SDK is currently providing initialization options http_proxy,https_proxy and relying on environment variables for fallback. If I want to bypass proxy config altogether, I have to set

sentry_sdk.init(
  https_proxy="",
  http_proxy="",
)

Which requires reading SDK logic for handling proxy (which may change in the future)

The current hack is also subtly different from

sentry_sdk.init(
  https_proxy=None,
  http_proxy=None,
)

(which will fallback to env vars and NOT what I want)

In my use case where sentry_sdk is deployed to many services, I don't have control over environment variables and need an option to disable/bypass proxy in Sentry SDK's internal transport.

Solution Brainstorm

File: sentry_sdk/consts.py

def __init__(
    self,
    # ... other parameters ...
    no_proxy=False,  # type: bool
    # ... other parameters ...
):

File: sentry_sdk/transport.py

def _in_no_proxy(self, parsed_dsn):
    # type: (Self, Dsn) -> bool
    if self.options["no_proxy"]:
        return True
    no_proxy = getproxies().get("no")
    if not no_proxy:
        return False
    # ... existing environment variable logic ...
sentrivana commented

Hey @scarletnguyen13, thanks for writing in. This sounds like a nice improvement. The snippets above look good to me to, do you feel like PR-ing them?

sentrivana commented

What we can also do (and thinking about it again, I think I'd prefer this way of doing things) instead of adding a new option is to allow http_proxy and https_proxy to be False, in which case the behavior would be to not use env vars.

scarletnguyen13 commented

Hmm I understand the reasoning for having them False, and that sounds good to me too, but just wondering if that would cause confusion for user (since we can have empty string, None, and False for the same options http_proxy and https_proxy)
What do you think? @sentrivana

sentrivana commented

Yeah that's also not ideal; also, having False just be an alias for "" is redundant. I think instead of piling stuff on top of the confusing default behavior actually the best way forward would be to document it better so that no one needs to figure out how to disable the proxy on their own. Will do this here: getsentry/sentry-docs#14816