Support for running Zipkin behind a reverse proxy #1732

stepanv · 2017-09-12T08:14:37Z

subfolder 'zipkin' is enforced
rewrite of the 'base' tag in the 'index.html' file is required

This implements the #1731

- subfolder 'zipkin' is enforced - rewrite of the 'base' tag in the 'index.html' file is required

codefromthecrypt · 2017-09-12T09:49:15Z

Thanks for taking a stab. Can you elaborate a little on how this leads to reverse proxy support, how it is tested, and if there are any README updates or otherwise needed? (ex in zipkin-ui for running locally)

stepanv · 2017-09-12T11:44:14Z

My use-case is quite simple, I need to run Zipkin in a different path than just /zipkin/.

The best solution would be to have all static links relative and let Javascript calculate the current url and not rely on /zipkin/.
I was able to implement the latter but the former was not that easy due to limitations of the html-webpack-plugin (see comments in the code).

With the changes I'm proposing, the good news is that if the reverse proxy supports rewriting of the static content (base tag and its href attribute) (e.g., Apache Httpd with enabled ProxyHTMLURLMap for the base tag, see https://httpd.apache.org/docs/2.4/mod/mod_proxy_html.html#proxyhtmlurlmap), the Zipkin works in any path such as /foo/bar/zipkin/.

It is of course possible to run Zipkin the same as before at the path /zipkin/, so this is not a breaking change.

Regarding tests and readme, I understand that both are important but I wanted to know how you feel about this change before updating the Docs. There are no tests ensuring that this functionality is preserved in future development. I propose to implement a single sanity integration test that Zipkin loads and that most common links work under a different path.

… relative URI is returned for a redirect to 'zipkin/' subpath.

codefromthecrypt · 2017-09-13T06:21:31Z

cc @naoman @devinsba I think you are also running under a proxy. feedback?

eirslett · 2017-09-13T07:32:50Z

How will this work with the browser plugin for Zipkin?

codefromthecrypt · 2017-09-13T08:31:07Z

cc + @SimenB for testing relating to the browser plugin

stepanv · 2017-09-13T09:15:20Z

By the way, as for the test error reported by CircleCI:

[ERROR] Tests run: 14, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 6.503 s <<< FAILURE! - in zipkin.junit.ZipkinRuleTest
[ERROR] postSpans_disconnectDuringBody(zipkin.junit.ZipkinRuleTest)  Time elapsed: 5.028 s  <<< ERROR!
java.io.IOException: unexpected end of stream on Connection{localhost:46744, proxy=DIRECT hostAddress=localhost/127.0.0.1:46744 cipherSuite=none protocol=http/1.1}
	at zipkin.junit.ZipkinRuleTest.postSpans(ZipkinRuleTest.java:276)
	at zipkin.junit.ZipkinRuleTest.postSpans_disconnectDuringBody(ZipkinRuleTest.java:197)
Caused by: java.io.EOFException: \n not found: limit=0 content=…
	at zipkin.junit.ZipkinRuleTest.postSpans(ZipkinRuleTest.java:276)
	at zipkin.junit.ZipkinRuleTest.postSpans_disconnectDuringBody(ZipkinRuleTest.java:197)

I'm unable to reproduce that locally. Isn't it possible it's an intermittently failing test? If so, are you able to instruct CircleCI to re-run the test/build? In the worst case I could add a dummy commit to re-run it.

codefromthecrypt · 2017-09-13T09:19:13Z

sorry about the flake. kicked

devinsba · 2017-09-13T12:36:47Z

Our reverse proxy setup is direct, we aren't modifying the paths at all so this won't affect me at the moment. But..

Can you put a blurb in the readme about the technical requirements for this to work? Also a sample apache config would probably be good. Thanks

eirslett · 2017-09-13T13:21:38Z

I would say the implementation itself isn't too bad, I imagined it would be more complex. Perhaps we should follow the rule-of-three?

naoman · 2017-09-13T22:30:03Z

@adriancole similar to @devinsba, we don't modify the path and are not planning to any time soon.

…bpack_public_path__' global variable.

stepanv · 2017-09-15T08:50:54Z

To answer the question regarding the Apache Httpd configuration for the reverse proxying.

(For now I'm not adding it to a readme, let me know if you have any suggestions where to put the information regarding reverse proxying.)

Lets assume we want to access Zipkin (running on localhost:9411) through Apache Httpd server (running on localhost:80 as a reverse proxy) in a context root /test/foo/zipkin/. No extra Zipkin configuration is required.

Apache Httpd configuration:

LoadModule proxy_module libexec/apache2/mod_proxy.so
LoadModule proxy_html_module libexec/apache2/mod_proxy_html.so
LoadModule proxy_http_module libexec/apache2/mod_proxy_http.so

ProxyPass /test/foo/ http://localhost:9411/
SetOutputFilter proxy-html
ProxyHTMLURLMap /zipkin/ /test/foo/zipkin/
ProxyHTMLLinks  base        href

To access Zipkin UI behind the reverse proxy, execute:

$ curl http://localhost/test/foo/zipkin/
<html><head><!--
      add 'base' tag to work around the fact that 'html-webpack-plugin' does not work
      with '__webpack_public_path__' being set as reported at https://github.com/jantimon/html-webpack-plugin/issues/119
    --><base href="/test/foo/zipkin/"><link rel="icon" type="image/x-icon" href="favicon.ico"><meta charset="UTF-8"><title>Webpack App</title><link href="app-94a6ee84dc608c5f9e66.min.css" rel="stylesheet"></head><body>
  <script type="text/javascript" src="app-94a6ee84dc608c5f9e66.min.js"></script></body></html>

As you can see, the attribute href of the base tag is rewritten which is the way to get around the html-webpack-plugin limitations.

Uploading the span is easy as

curl -H "Content-Type: application/json" --data-binary "[$(cat /path/to/zipkin/benchmarks/src/main/resources/span-local.json)]" http://localhost/test/foo/api/v1/spans

And then it's of course observable in the UI:

open http://localhost/test/foo/zipkin/?serviceName=zipkin-server&startTs=1378193040000&endTs=1505463856013

stepanv · 2017-09-15T09:02:39Z

By the way, in the latest commit I got rid of the __webpack_public_path__ everywhere. Instead, a publicPath module is used to import contextRoot variable that is referenced in all the URIs.

eirslett · 2017-09-15T10:05:32Z

How much effort would it take to fix the issue in html-webpack-plugin?

By the way, in the latest commit I got rid of the webpack_public_path everywhere. Instead, a publicPath module is used to import contextRoot variable that is referenced in all the URIs.

Sounds like a good idea, so we don't leak webpack-specific stuff everywhere in the code base

eirslett · 2017-09-15T10:08:27Z

zipkin-ui/js/publicPath.js

+// read the public path from the <base> tag where it has to be set anyway because of
+// html-webpack-plugin limitations: https://github.com/jantimon/html-webpack-plugin/issues/119
+// otherwise it could be: window.location.pathname.replace(/(.*)\/zipkin\/.*/, '$1/zipkin/')
+const contextRoot = $('base').attr('href');


Should we sanity check that it ends with /? Looks like the rest of the code assumes that. We could either show an error to the user, or just append / here?

You're right, I'll append / if missing.

eirslett · 2017-09-15T10:10:45Z

zipkin-ui/js/component_data/default.js

@@ -21,7 +22,7 @@ export default component(function DefaultData() {
    const query = convertToApiQuery(window.location.search);
    const serviceName = query.serviceName;
    if (serviceName) {
-      const apiURL = `/zipkin/api/v1/traces?${queryString.stringify(query)}`;
+      const apiURL = `${contextRoot}api/v1/traces?${queryString.stringify(query)}`;


There's a lot of code duplication going on here... maybe we should make a apiClient.js which basically just accepts relative paths and wraps $.ajax?

Seems reasonable. I'll take a look at it.

Done. As a matter of fact, the $.ajax call with relative paths use the <base> tag and as such there was no need to wrap it as you suggested.

stepanv · 2017-09-15T10:39:47Z

@eirslett, to be honest, I wanted to avoid even looking at the html-webpack-plugin codebase. Even if it was easy, it would take weeks to have the feature I need in Zipkin working.
The base tag is not an ugly work around, it's a way to go, clean and simple, that happens to work well even for the html-webpack-plugin. It puts a constraint on the reverse proxy, to rewrite the tag, but that's such a common behavior. For me it's a reasonable trade-off.

stepanv · 2017-09-15T10:53:50Z

And the fact that html-webpack-plugin devs don't bother that it doesn't conform with the webpack spec (jantimon/html-webpack-plugin#119) is quite discouraging for me, someone who's not from the JS world..

…ase' html). Appending '/' to the 'contextRoot' if not present.

stepanv · 2017-09-20T09:14:58Z

I pushed new changes.
If you think it's ok, please let me know what kind of tests and documentation you require in order to accept the pull request.

codefromthecrypt · 2017-10-02T02:59:37Z

sorry about the delay. trying now. I really like this, actually. a lot tighter than I expected it to be.

codefromthecrypt · 2017-10-02T03:34:53Z

tested and looks very good. I'd add your comments about how to run in apache as an README.md section.

A favor.. we broke local dev mode at some point. Can you help fix proxy=http://localhost:9411 ./npm.sh run dev or similar?

Fixed running in a 'devServer' mode. Assuming the 'zipkin-server' runs at 'localhost:9411', execute: proxy=http://localhost:9411/zipkin/ ./npm.sh run dev

stepanv · 2017-10-02T13:02:02Z

@adriancole, I pushed another commit with extended README.md and fixed devServer.
Just note that I had to add /zipkin/ context root to the proxy env var to make it work (I hope it's ok):

proxy=http://localhost:9411/zipkin/ ./npm.sh run dev

codefromthecrypt · 2017-10-03T00:58:14Z

very nice work.

* Support for running Zipkin behind a reverse proxy - subfolder 'zipkin' is enforced - rewrite of the 'base' tag in the 'index.html' file is required * Fixed 'redirectedHeaderUsesOriginalHostAndPort' test to ensure that a relative URI is returned for a redirect to 'zipkin/' subpath. * Using 'contextRoot' as a code style conforming reference to the '__webpack_public_path__' global variable. * Using relative URIs for AJAX calls (which works thanks to the 'head>base' html). Appending '/' to the 'contextRoot' if not present. * Added readme regarding running behind a reverse proxy. Fixed running in a 'devServer' mode. Assuming the 'zipkin-server' runs at 'localhost:9411', execute: proxy=http://localhost:9411/zipkin/ ./npm.sh run dev

Support for running Zipkin behind a reverse proxy

24a6ecf

- subfolder 'zipkin' is enforced - rewrite of the 'base' tag in the 'index.html' file is required

Fixed 'redirectedHeaderUsesOriginalHostAndPort' test to ensure that a…

59594e4

… relative URI is returned for a redirect to 'zipkin/' subpath.

Using 'contextRoot' as a code style conforming reference to the '__we…

8ab814e

…bpack_public_path__' global variable.

eirslett reviewed Sep 15, 2017

View reviewed changes

Using relative URIs for AJAX calls (which works thanks to the 'head>b…

ad4d38b

…ase' html). Appending '/' to the 'contextRoot' if not present.

codefromthecrypt mentioned this pull request Sep 26, 2017

Hosting zipkin UI through a proxy #1452

Closed

Added readme regarding running behind a reverse proxy.

6aea223

Fixed running in a 'devServer' mode. Assuming the 'zipkin-server' runs at 'localhost:9411', execute: proxy=http://localhost:9411/zipkin/ ./npm.sh run dev

codefromthecrypt merged commit adb8693 into openzipkin:master Oct 3, 2017

stepanv mentioned this pull request Feb 22, 2018

Allow customization of UI base tag for running behind proxies #1731

Closed

momentum123 mentioned this pull request Apr 13, 2018

zipkin broken ui thru gateway #1993

Closed

codefromthecrypt added enhancement ui Zipkin UI labels Oct 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for running Zipkin behind a reverse proxy #1732

Support for running Zipkin behind a reverse proxy #1732

stepanv commented Sep 12, 2017

codefromthecrypt commented Sep 12, 2017

stepanv commented Sep 12, 2017

codefromthecrypt commented Sep 13, 2017

eirslett commented Sep 13, 2017

codefromthecrypt commented Sep 13, 2017

stepanv commented Sep 13, 2017

codefromthecrypt commented Sep 13, 2017 via email

devinsba commented Sep 13, 2017

eirslett commented Sep 13, 2017

naoman commented Sep 13, 2017 •

edited

Loading

stepanv commented Sep 15, 2017 •

edited

Loading

stepanv commented Sep 15, 2017

eirslett commented Sep 15, 2017 •

edited

Loading

eirslett Sep 15, 2017

stepanv Sep 15, 2017

stepanv Sep 20, 2017

eirslett Sep 15, 2017

stepanv Sep 15, 2017

stepanv Sep 20, 2017

stepanv commented Sep 15, 2017 •

edited

Loading

stepanv commented Sep 15, 2017

stepanv commented Sep 20, 2017

codefromthecrypt commented Oct 2, 2017

codefromthecrypt commented Oct 2, 2017

stepanv commented Oct 2, 2017

codefromthecrypt commented Oct 3, 2017

Support for running Zipkin behind a reverse proxy #1732

Support for running Zipkin behind a reverse proxy #1732

Conversation

stepanv commented Sep 12, 2017

codefromthecrypt commented Sep 12, 2017

stepanv commented Sep 12, 2017

codefromthecrypt commented Sep 13, 2017

eirslett commented Sep 13, 2017

codefromthecrypt commented Sep 13, 2017

stepanv commented Sep 13, 2017

codefromthecrypt commented Sep 13, 2017 via email

devinsba commented Sep 13, 2017

eirslett commented Sep 13, 2017

naoman commented Sep 13, 2017 • edited Loading

stepanv commented Sep 15, 2017 • edited Loading

stepanv commented Sep 15, 2017

eirslett commented Sep 15, 2017 • edited Loading

eirslett Sep 15, 2017

Choose a reason for hiding this comment

stepanv Sep 15, 2017

Choose a reason for hiding this comment

stepanv Sep 20, 2017

Choose a reason for hiding this comment

eirslett Sep 15, 2017

Choose a reason for hiding this comment

stepanv Sep 15, 2017

Choose a reason for hiding this comment

stepanv Sep 20, 2017

Choose a reason for hiding this comment

stepanv commented Sep 15, 2017 • edited Loading

stepanv commented Sep 15, 2017

stepanv commented Sep 20, 2017

codefromthecrypt commented Oct 2, 2017

codefromthecrypt commented Oct 2, 2017

stepanv commented Oct 2, 2017

codefromthecrypt commented Oct 3, 2017

naoman commented Sep 13, 2017 •

edited

Loading

stepanv commented Sep 15, 2017 •

edited

Loading

eirslett commented Sep 15, 2017 •

edited

Loading

stepanv commented Sep 15, 2017 •

edited

Loading