-
Notifications
You must be signed in to change notification settings - Fork 52
Add proxy support for corporate environments #209
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
Conversation
|
Thank you for the contribution! On the surface, this already looks great, I'll dive into the details later. One question upfront though: Do we need |
we could use https-proxy-agent. However I chose to use |
Thanks, didn't know that - I also just checked, and you're right, Using this as a proxy is the way to go: https://undici.nodejs.org/#/docs/api/ProxyAgent?id=example-basic-proxyagent-instantiation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conditional approval for the proposed changes 👍
Still to be done:
- Move the text in README.md a couple of lines towards the bottom, so it's not in between the config description for
DT_ENVIRONMENT - Consider removing the
shouldBypassProxyhelper, as it's not used
edit: Please sign the CLA
Issues addressed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@manoharnv Thank you for the changes ;) |
b3a3a3e to
afe3dae
Compare
- Created proxy-config utility to configure undici ProxyAgent - Added support for https_proxy, HTTPS_PROXY, http_proxy, HTTP_PROXY, no_proxy, and NO_PROXY environment variables - Proxy is configured early in startup process before any HTTP requests - Added comprehensive test coverage for proxy configuration - Updated README.md with proxy environment variable documentation - Updated CHANGELOG.md to document new feature Co-authored-by: manoharnv <[email protected]>
- Remove unnecessary log message when no proxy is configured - Clarify no_proxy limitation in code comments and README - Update .env.template with proxy configuration examples Co-authored-by: manoharnv <[email protected]>
Fixing as per review comment. placing the proxy configuration at the right place.
Co-authored-by: manoharnv <[email protected]>
afe3dae to
1bb62aa
Compare
e7ea62d
into
dynatrace-oss:main
Problem
The Dynatrace MCP Server was not honoring system proxy settings (
https_proxy,HTTPS_PROXY,no_proxy,NO_PROXYenvironment variables), causing connection failures when running behind corporate proxies. This prevented users in enterprise environments from using the MCP server to connect to their Dynatrace environments.Solution
This PR implements proxy support by configuring undici's
ProxyAgentearly in the application startup process, before any HTTP requests are made. The solution leverages undici (already available as a transitive dependency viadt-app), which is the underlying implementation of Node.js's globalfetchAPI.Key Changes
New proxy configuration utility (
src/utils/proxy-config.ts):https_proxy,HTTPS_PROXY,http_proxy,HTTP_PROXY)no_proxy/NO_PROXYfor documenting bypass hosts (parsed and logged)Integration (
src/index.ts):configureProxyFromEnvironment()early in the startup processDocumentation:
Testing
Usage Example
Expected output:
Known Limitation
The
no_proxyenvironment variable is currently parsed and logged for informational purposes but not fully enforced by undici's ProxyAgent. This is a limitation of the underlying HTTP client library. Users who need no_proxy functionality should configure their proxy servers to handle these exclusions. This limitation is clearly documented in both code comments and user-facing documentation.Impact