newrelic / node-newrelic

New Relic Node.js agent code base. Developers are welcome to create pull requests here, please see our contributing guidelines. For New Relic technical support, please go to http://support.newrelic.com.
Apache License 2.0
971 stars 399 forks source link

Running the agent behind a proxy #128

Closed EdJ closed 10 years ago

EdJ commented 10 years ago

Currently the New Relic agent is not functional from behind a proxy server.

Setting the NEW_RELIC_PROXY_HOST and NEW_RELIC_PROXY_PORT environment variables switches the agent from failing due to inability to connect to an issue with (what looks like) an inability to connect to local storage.

Full stack trace:

New Relic for Node.js was unable to start due to an error:
Error: connect EINVAL
    at errnoException (net.js:904:11)
    at connect (net.js:766:19)
    at net.js:845:9
    at <appDir>/node_modules/newrelic/node_modules/continuation-local-storage/node_modules/async-listener/glue.js:177:31
    at asyncCallback (dns.js:68:16)
    at Object.onanswer [as oncomplete] (dns.js:121:9)

Can you confirm whether any work is currently being undertaken on this issue (as it is noted within lib/config.default.js, and has been for a number of versions); if not am I OK to take a look and submit a pull request if it is fixable without significant changes?

othiym23 commented 10 years ago

I think I added some text to lib/config.default.js in v1.4.0 to indicate that proxy support is straight-up not working right now; it occurs to me that that would have been more useful in the README. Sorry!

Proxy support is high on the roadmap! We originally planned to release it as part of the SSL support, but SSL proved to be more involved than we'd anticipated, and SSL needed to be in place so browser monitoring could be released, so we deferred proxy support. If you want to take a stab at it, I can't stop you, but there are a number of subtleties, especially around SSL and testing, that make it something like actual work to get functional. We're rejiggering the roadmap right now, but my guess is that proxy support will be finished / fixed before too long. It's still important. Sorry for the incomplete feature and vagueness in the documentation, and thanks for your patience!

EdJ commented 10 years ago

No problem, it's pretty obvious once I figured out where to look for the settings!

I have no problem justifying taking some time to look at it properly; as we are in an on-site datacentre setup we are unfortunately forced to use a proxy server (which has caused a number of issues with various monitoring tools that run over HTTP).

I will throw in a PR if I get anywhere. Are there any specifics that I should look to deal with, specifically the issues with testing you alluded to?

othiym23 commented 10 years ago

A lot of the trickiness is around SSL -- there are a lot of settings that the module uses to lock down HTTPS, and getting that working properly with proxies will b a fair amount of work, as well as testing and verifying that the module is doing the right thing. If you're just looking for plain HTTP proxying, it's just a matter of making sure that all the CONNECT method stuff is handled correctly, preferably with keep-alive being passed through. Let us know how it goes!

EdJ commented 10 years ago

This is an interesting issue. I have created a couple of functioning tests that use a local node http-proxy to replicate a proxy setup and the API connection seems to be completely functional (on HTTP). Trying the same test in HTTPS mode, does, as you'd suggested, not work (timeouts etc., there is quite a bit of certificate handling that's causing issues).

However, when running the actual agent I'm seeing the same issue as previously outlined, which seems to just be a clamped stacktrace from the continuation-local-storage module (specifically its dependency async-listener). I think the base issue may actually lie within newrelic/yakaa, and in order to check whether this is the case I switched yakaa out for tootallnate/node-http-proxy-agent, which does not implement keep-alive correctly but is a proxy-capable http agent. I have yet to get a CONNECT request to work correctly out to the API via this proxy, although I am getting a response:

{"exception":{"message":"Server Error (EOFException): Unexpected end of ZLIB input stream","error_type":"RuntimeError"}}

Which would point to the wrong request type, or an issue with the proxy forwarding the headers / content.

I will focus my efforts on extending yakaa for the time being, assuming I can justify continuing to look at this issue on Monday.

(n.b. I'm sure you would have arrived to these conclusions in a shorter timeframe than I, however, it took me a while to figure out that the /lib/collector/http-agent.js file seemed to be the root, not lib/collector/remote-method.js)

othiym23 commented 10 years ago

You have to be careful about the headers you set on your request -- see the internals of lib/collector/remote-method.js in _getHeaders for some of the details. In particular, the exception you're getting right now leads me to believe that the Content-Encoding header isn't getting proxied through. It needs to be either identity or deflate, or the New Relic backend gets confused.

In general, the SSL issues I'm concerned about are related to the fact that New Relic for Node has pretty stringent security requirements when it's making its own collection (as you've already seen, if you've stumbled across lib/collector/http-agents.js). Getting the tunnel set up correctly is going to be difficult if you don't have control over both the TLS connection to the proxy and the proxied TLS connection to New Relic, and that's the part that I've always assumed would be tricky to test.

Be aware that the reason for yakaa is because of the need to get working pipelining over a TLS socket in versions of Node pre-0.12. Because New Relic for Node deprecates RC4 to such a low level, TLS session creation is more expensive, and pipelining is necessary in order to keep our TLS terminators from getting slammed with load. Maybe Nate's module handles the Keep-Alive issue properly, but a surprising number of contenders I looked at didn't.

Good luck! I don't know when New Relic is actually going to get to this, and I appreciate the effort you've put into this so far. Sorry I haven't been able to help you out more tangibly!

EdJ commented 10 years ago

Things are plodding along nicely; I have proxying working on HTTP and have correctly got a tunnelling HTTPS connection implemented, using koichik/node-tunnel. I'm now in the position of trying to work out how to (reliably) test this, as well as how to unify the interface for both tunnelled and non-tunnelled HTTPS connections, as they operate fairly differently (for example, the request options are the standard non-proxied options when tunnelled).

I am thinking the best way to go about this is to adjust the implementation of lib/collector/http-agents.js, most likely by providing it with the configuration to allow it to decide which agent to provide to the user (making it a kind of "agent factory"). This way we can keep yakaa, but replace it with node-tunnel if a proxy is required. Does this sound like a reasonable solution? On the other hand, with the only other place where proxy details are critical being within lib/collector/remote-method.js, it may be better to have the slight config changes be applied only there, and have lib/collector/http-agents.js expose more agents with proxy and non-proxy options.

Also, I'd appreciate it if you'd cast an eye over node-tunnel and ensure it meets your security requirements. I'm expecting that many people will be using HTTPS over HTTP tunnelling, which I don't believe requires such fine-grained control over the connections. I'm also not sure how you'd get a proxy to enforce the security requirements when negotiating the tunnel, at least not via the client, any suggestions?

EdJ commented 10 years ago

Currently midway through implementation, if you want to take a look checkout https://github.com/EdJ/node-newrelic

othiym23 commented 10 years ago

Thanks for the updates on your progress, and I'll take a look!

It makes sense for the tunneling / proxying behavior to be encapsulated in the agent, so at some point we'll fork yakaa into yapkaa (Yet Another Proxying Keep-Alive Agent) and put everything there, but until everything is sorted out (and we've gotten to the root of an obnoxious SSL error a few customers have been seeing), developing the two forks independently makes sense.

Preserving request pipelining is an important design goal, because New Relic's back end handles a LOT of traffic, and Node's TLS session caching support is nonexistent, which makes Node kind of an obnoxious client for our back end. But what you're doing sounds reasonable for now.

EdJ commented 10 years ago

Have just managed to come back to this after being pulled onto some other high-priority delivery work for the last week or so. As expected the key issue here is not the implementation itself, but the testing. My current branch seems to implement keep-alive'd TLS tunnelling "properly", or at least I have verified that it is re-using the agent correctly and the connection is running over HTTPS. I am looking at getting HTTPS over HTTPS working (currently it's HTTPS over HTTP) as I had a couple of issues with this.

Automating testing the implementation is extremely difficult. Nock exists at a level slightly ABOVE the usage of HTTP agents, and so cannot intercept, or even perceive, the initial CONNECT request, or the fact that the agent requests are being tunnelled. Getting an integration level test working is somewhat simpler, but certificates become something of an issue here. I'd prefer not to have to resort to using proxyquire (or something similar), since you're already using nock and it would be less life-like, so do you have any suggestions as to where I might go with this? Agents really are a very poorly documented (and, by the seems of things, rarely-utilised) aspect of the core...

othiym23 commented 10 years ago

If it were me doing what you're doing, I'd probably just sigh and use Sinon mocks to do the lower-level request mocking. nock is so much easier to deal with, but as you said, it's not designed for this use case.

For integration tests, it makes sense to make the certificates in use more directly injectable, especially because it seems likely that customers like you are going to need more control over certificates, at least when connecting to their own proxies. I also do think that it makes sense to move proxy support into the agent itself by forking yakaa into yapkaa, but that's just me thinking about design, and not saying that needs to happen right now.

The agent interface is indeed poorly documented, but the profusion of mostly-identical agents in npm indicates that they're probably more widely-used than you might think. Node's docs just need help in general, but who has the time?

We're working towards releasing a new version of newrelic that will include an important fix to yakaa (which is on the yakaa repo now, but still unpublished), and maybe after that your stuff will be finished enough that we can start to look at integrating it into the module. Thanks for giving this so much attention!

EdJ commented 10 years ago

Ok, I've put in a "slightly icky" test that ensures the new method I added to http-agents.js is called when the proxy is set and it uses HTTPS. I've also had to add a config variable to notify whether to use HTTPS over HTTP or HTTPS over HTTPS. Unfortunately, due to my company's proxy not supporting HTTPS connections (we can only use HTTPS over HTTP) I can't easily test the second configuration in a live environment. I will see what I can do about getting an integration test that tests both.

I agree it makes sense to integrate the tunnelling into yakaa (for security reasons, mostly), but for now I have pulled in node-tunnel as it does the job just fine.

EdJ commented 10 years ago

Right, I've pretty much got this to a place where I'm (mostly, probably 90%) happy with it. There are a couple of unit tests checking the SSL proxy setup (hand-rolled a quick stub for the http-agents property I added that serves the proxy agent), and there are two integration tests that show the agent tunnelling via a proxy correctly. It also supports SSL and non-SSL proxyies, although I had some issues getting a self-signed certificate working properly in the SSL test (this could do with fixing), ending up simply turning off the check for certificate validity. As my company use an HTTP-only proxy (...) I cannot justify spending any further time trying to get to the bottom of the certificates issue.

I have checked that these modifications work inside a live app (they do), so I guess really I could just do for you to take a look at the fork and make sure the code / tests / etc. are up to your standards. If so, I will update the docs and do a pull request (assuming this is your PR workflow).

othiym23 commented 10 years ago

So today is my last day at New Relic, @EdJ, and I'm sorry that I didn't have time to get through the work you've done. However! I still intend to add proxy support to yakaa (probably forking it to be named yapkaa, just to keep yapkaa itself relatively simple). It will be up to @groundwater and @wraithan to integrate your work back into the New Relic code base, but I really appreciate you taking the time to document your progress, and I'm pretty hopeful that New Relic will be able to take advantage of what you've done. Thanks! And thanks for your patience!

EdJ commented 10 years ago

Not a problem; best of luck with whatever new opportunity is on your plate :) Appreciated the guidance you've given on the matter. FYI we are now running my branch in our sandbox environment at work, and everything is looking fine (somewhat ironically, my last day at that workplace is next Friday !). As soon as someone gets a chance to look at the branch I'll be happy to pitch in again (one thing that I have realised is missing is proxy authentication, for example...) to make sure the agent gets updated.

On Fri, Apr 18, 2014 at 10:28 PM, Forrest L Norvell < notifications@github.com> wrote:

So today is my last day at New Relic, @EdJ https://github.com/EdJ, and I'm sorry that I didn't have time to get through the work you've done. However! I still intend to add proxy support to yakaahttps://github.com/newrelic/yakaa(probably forking it to be named yapkaa, just to keep yapkaa itself relatively simple). It will be up to @groundwater https://github.com/groundwater and @wraithanhttps://github.com/wraithanto integrate your work back into the New Relic code base, but I really appreciate you taking the time to document your progress, and I'm pretty hopeful that New Relic will be able to take advantage of what you've done. Thanks! And thanks for your patience!

— Reply to this email directly or view it on GitHubhttps://github.com/newrelic/node-newrelic/issues/128#issuecomment-40846095 .

groundwater commented 10 years ago

Beta proxy support is available in v1.7.5.

I recommend setting the proxy property in your newrelic.js to the proxy URL. This should support sending to newrelic with ssl enabled.

You will need to enable this behavior via a feature flag for now:

// inside your newrelic.js

ssl: true,

proxy: 'http://myproxy:3182',

feature_flag: {
  proxy: true,
},

We recommend only running beta features in staging/development, but would appreciate any feedback you have.

qburst-praven commented 10 years ago

hi there,

I've tried the same using 1.7.5 - and am still not able to connect via the proxy. It works fine in 1.3.0 - bt not in any of the higher versions...

The following is my newrelic.js config with 1.7.5 -

exports.config = {
  app_name : ['XXXXXXXXXX'],
  license_key : 'XXXXXXXXX',
  ssl: true,
  proxy_host : 'http-proxy.XXXX.com',
  proxy_port : '80',
  feature_flag: {
    proxy: true,
  },
  logging : {
    level : 'trace'
  }
};

Regards Praven John

groundwater commented 10 years ago

Hi @PravenJohn, do you have more information?

  1. Does your proxy accept the http CONNECT method? What do the following output?

    curl -i -x http://http-proxy.XXXX.com:80/ https://collector.newrelic.com

    vs

    curl -i -x http://http-proxy.XXXX.com:80/ http://collector.newrelic.com
  2. Can you send us your newrelic_agent.log file.

The proxy connection protocol differs when using SSL to connect to your destination. The 1.3.0 agent would have used the simpler http protocol because we did not have ssl support.

Thanks for trying out beta proxy support, we'll try to get you running as soon as possible.

qburst-praven commented 10 years ago
[root@cihcissapp893v ~]# curl -i -x http://http-proxy.XXXX:80/ https://collector.newrelic.com
HTTP/1.0 200 Connection established
via: HTTP/1.1 proxy1451

HTTP/1.1 400 Bad Request
Content-Length: 0

Vs

[root@cihcissapp893v ~]# curl -i -x http://http-proxy.XXXX:80/ http://collector.newrelic.com
HTTP/1.1 400 Bad Request
Date: Mon, 07 Jul 2014 17:15:36 GMT
Content-Length: 0
via: HTTP/1.1 proxy1451
Proxy-Connection: Keep-Alive
Connection: Keep-Alive

I cant attach the log directly here - since it is againist our company's policies - bt I could send it to you directly if that helps?

groundwater commented 10 years ago

@PravenJohn jacob@newrelic.com

qburst-praven commented 10 years ago

just mailed u the same - I'm leaving for the day - I look forward to hearing back from you...

groundwater commented 10 years ago

It strikes me now that I should add more logging to proxy support :smile:

qburst-praven commented 10 years ago

tried 1.8.0 too - and no luck...

dijam commented 10 years ago

We had similar issues with proxy. We used proxy instead of proxy_host and proxy_port. That solved our issue and we can now get data in newrelic. We used to get this: http://collector.newrelic.com:80http://collector.newrelic.com:80/agent_listener/invoke_raw_method... And after switching to proxy="http://proxy:port" it started working. Hope this help.

txase commented 10 years ago

Hi @PravenJohn and @dijam,

We're sorry you still have some issues with our proxy support. We are in the process of closing down our usage of GitHub issues. If you are still having issues, please follow up with us at http://support.newrelic.com. We are better able to support you through our dedicated portal. We appreciate your understanding as we undergo this transition.