ZIPKIN_UI_BASEPATH doesn't seem to work with 3.1.0 #3742
Comments
|
@ujo-trackunit so ZIPKIN_UI_BASEPATH only rewrites links so that when a proxy has zipkin under another path, the links reflects the expectations of the proxy. zipkin-server will still host everything under /zipkin I'm not saying there isn't an issue with the link rewriting, but that's what the feature was for.. https://github.com/openzipkin/zipkin/tree/master/zipkin-lens#running-behind-a-reverse-proxy |
|
I started to look at something similar, but I think it is different than your issue. Our proxy-example extracted the UI assets to the wrong directory which caused 404s.. #3743 |
|
@ujo-trackunit can you mention the version you were moving from? I'd like to try to isolate what's going on.. |
|
We are running 3.0.6 |
|
What I am seeing is that stuff is not kept under |
|
ok I think maybe found it.. $ docker run --rm -e ZIPKIN_UI_BASEPATH=/admin/zipkin -p 9411:9411 openzipkin/zipkin:3.0.6
$ curl -s localhost:9411/zipkin/index.html |tidy -q -i --tidy-mark no --wrap 1024
line 1 column 22 - Warning: inserting missing 'title' element
<!DOCTYPE html>
<html>
<head>
<base href="/admin/zipkin/">
<meta charset="utf-8">
<link rel="icon" href="./favicon.ico">
<script defer="defer" src="./static/js/main.c84b9771.js"></script>
<link href="./static/css/main.311b483b.css" rel="stylesheet">
<title></title>
</head>
<body>
<noscript>You need to enable JavaScript to run this app.</noscript>
<div id="root"></div>
</body>
</html>Now, the href links are not relative (also we didn't strip comments but that's not a big deal) $ docker run --rm -e ZIPKIN_UI_BASEPATH=/admin/zipkin -p 9412:9411 openzipkin/zipkin:3.1.0
$ curl -s localhost:9412/zipkin/index.html |tidy -q -i --tidy-mark no --wrap 1024
curl -s localhost:9412/zipkin/index.html |tidy -q -i --tidy-mark no --wrap 1024
line 18 column 3 - Warning: inserting missing 'title' element
<!--
Copyright 2015-2024 The OpenZipkin Authors
Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
in compliance with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software distributed under the License
is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
or implied. See the License for the specific language governing permissions and limitations under
the License.
-->
<!DOCTYPE html>
<html>
<head>
<base href="/admin/zipkin/">
<meta charset="utf-8">
<link rel="icon" href="/zipkin/favicon.ico">
<script type="module" crossorigin="" src="/zipkin/static/js/index.51c4d5b0.js"></script>
<link rel="stylesheet" href="/zipkin/static/css/index.a7aa86b4.css">
<title></title>
</head>
<body>
<noscript>You need to enable JavaScript to run this app.</noscript>
<div id="root"></div>
</body>
</html> |
|
so I think the following change gets it working again, but it also breaks the zipkin-ui docker demo ;) cc @openzipkin/ui as while I tend to pull a large share of troubleshooting, probably more healthy if someone else helps. basically, we both want to have We don't have any integration tests for either, closest is that --- a/zipkin-lens/vite.config.ts
+++ b/zipkin-lens/vite.config.ts
@@ -17,9 +17,9 @@ import {defineConfig} from 'vite'
import * as path from "path";
// baseUrl is the default path to lookup assets.
-const baseUrl = process.env.BASE_URL || '/zipkin';
+const baseUrl = process.env.BASE_URL || './';
// basePath is the default path to get dynamic resources from zipkin.
-const basePath = process.env.BASE_PATH || baseUrl;
+const basePath = process.env.BASE_PATH || '/zipkin';
const zipkinProxyConfig = {
target: process.env.API_BASE || 'http://localhost:9411',
|
|
cc also @SamTV12345 in case you are around. you can see the above notes on what's the dillemma. it is stuff like this why I tend to cringe at adding more variables like more UI builds in other projects. Post merge, when things break, there are few if any around to help. |
|
unmarking myself from this as I'm pulling too many hours here and not enough on day job. 🤞 someone else can rescue this time! |
|
@reta @anuraaga I'm actually inclined to roll back lens to react-scripts and re-raise vite progress here on another branch until it is fully tested and works on all scenarios we supported before. The only reason we changed to vite was because of dev dependency CVE, which is not a good reason to break production users. We can then take more time, and possibly we can all learn from this about merging change to major pieces without testing everything in and outside docker. There were several PRs after the first commit to get things working and we still aren't done. This is a lot of human effort for a non-prod CVE, even if we re-merge it later. Thoughts? |
I'll give it a try. Sorry for your frustration over the frontend changes. Maybe I can come up with a solution this evening or I'll try it this weekend. |
@SamTV12345 the main frustration is that I am not good at it, so for example, it takes as long as me to troubleshoot something simple on the JS side vs supporting a new version of cassandra or something. Also, each and any frontend related PR tends to need a lot of feedback to merge, which again is the same effort I would usually spend on a new feature. Sometimes over several days.. So there's displacement and a lot of time with each thing, often unplanned because I'm not aware there is a problem until someone notices.. Thanks for giving it a stab, please do run the docker/examples/README example for the UI after the fix to make sure this still works. I'll do that when you say it is working as well. Once whatever is merged that doesn't break the UI example, the ghcr.io/openzipkin/zipkin:master tag can be updated and @ujo-trackunit can verify. If it turns out we can't sort this by early next week, I will raise a PR to revert vite and make a patch release that gets the UI working again, then raise a shared branch with vite again. This is surely not my preference, as it makes more work in the future to finish that. However, we can't leave this broken as it was a feature for many years. |
|
@codefromthecrypt I've got a fix ready for review and testing. The paths are now correctly rewritten. |
|
@SamTV12345 thanks. I'll leave comments but looks reasonable! |
|
@ujo-trackunit can you verify this is ok for you? once you say yes I'll help cut a release tag $ docker run --pull always --rm -e ZIPKIN_UI_BASEPATH=/admin/zipkin -p 9411:9411 ghcr.io/openzipkin/zipkin:master$ curl -s localhost:9411/zipkin/index.html |tidy -q -i --tidy-mark no --wrap 1024
<!--
Copyright 2015-2024 The OpenZipkin Authors
Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
in compliance with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software distributed under the License
is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
or implied. See the License for the specific language governing permissions and limitations under
the License.
-->
<!DOCTYPE html>
<html>
<head>
<base href="/admin/zipkin">
<meta charset="utf-8">
<link rel="icon" href="/admin/zipkin/favicon.ico">
<title>Zipkin</title>
<script type="module" crossorigin="" src="/admin/zipkin/static/js/index.51c4d5b0.js"></script>
<link rel="stylesheet" href="/admin/zipkin/static/css/index.a7aa86b4.css">
</head>
<body>
<noscript>You need to enable JavaScript to run this app.</noscript>
<div id="root"></div>
</body>
</html> |
|
Hi - still seeing some issues - running with same config as for 3.0.6 - but seeing:
Missing some static resources and the "Run query" bar. This is a screen shot from 3.0.6:
|
|
ok before, the other assets were looked up relative like this. I'm not sure why now they are absolute, but perhaps at least when baseUrl is set, we can override it to relative. I'll raise a test PR we can revert if it doesn't fix things.. #3742 (comment) |
|
@ujo-trackunit can you try again please? |
|
I think this never worked properly as it was never tested except ad-hoc. I have an httpd container in progress locally which needs a couple changes to fully work. will update |
|
very close, just one more glitch hopefully #3751 |
|
Testing with |
|
Thanks @ujo-trackunit I know that #3751 is part of the fix, just I noticed a glitch where if you click on something like Dependencies link, it will revert the URL back to /zipkin (even though it works, this is a problem for bookmarking). So, I'll ping you again once we fix the last dent. |
|
@ujo-trackunit sorry this took so long. It has been on my mind every day, and 🤞 this is the last one. I created an example that should have been added several years ago when that env variable was documented. That was my fault as it was on my watch back then. Anyway it is here now https://github.com/openzipkin/zipkin/tree/master/docker/examples#ui-proxy I started the new example which defaults to /admin/zipkin $ cd docker/examples
$ TAG=master docker-compose -f docker-compose.yml -f docker-compose-uiproxy.yml pull
$ TAG=master docker-compose -f docker-compose.yml -f docker-compose-uiproxy.yml upThen, I checked the UI at http://localhost/admin/zipkin/ and it worked and also curl looks sane, too $ curl -s localhost:9411/zipkin/index.html |tidy -q -i --tidy-mark no --wrap 1024
<!--
Copyright The OpenZipkin Authors
SPDX-License-Identifier: Apache-2.0
-->
<!DOCTYPE html>
<html>
<head>
<base href="/admin/zipkin/">
<meta charset="utf-8">
<link rel="icon" href="./favicon.ico">
<title>Zipkin</title>
<script type="module" crossorigin="" src="./static/js/index.4b42884d.js"></script>
<link rel="stylesheet" href="./static/css/index.a7aa86b4.css">
</head>
<body>
<noscript>You need to enable JavaScript to run this app.</noscript>
<div id="root"></div>
</body>
</html>If you can try one more time pulling the master tag, let's see if it sorts you. If not, please paste the curl output you get. Thanks again for your time and patience! |
|
@codefromthecrypt I am sorry, my previous test was moot - it was the latest build. Tested with: and it looks good - also did searches and filtering:
Thank you for looking into this. |
|
3.1.1 going out now with this fixed! Thanks, all! |
We have upgraded to 3.1.0. We are using
ZIPKIN_UI_BASEPATHto rewrite the base path for the UI, but it doesn't seem to work anymore. Tried running:and the UI shows nicely on http://localhost:9411 (redirects to
http://localhost:9411/zipkin/). Withit still redirects to
http://localhost:9411/zipkin/but shows blank page. If I trylocalhost:9411/admin/zipkin/I get404 Not Found.Perhaps I am missing some configuration changes for the upgrade?
The text was updated successfully, but these errors were encountered: