Skip to content
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

lens: adds zipkin-uiproxy image and fixes ZIPKIN_UI_BASEPATH #3751

Merged
merged 6 commits into from
Mar 6, 2024

Conversation

codefromthecrypt
Copy link
Member

@codefromthecrypt codefromthecrypt commented Mar 6, 2024

This adds a new test image zipkin-uiproxy which allows us to verify the ZIPKIN_UI_BASEPATH. This allowed me to notice and fix the following glitches since we migrated to vite:

  • ensure basePath has a trailing slash in the index.html
  • fix bug where we didn't use BASE_PATH in App.tsx
  • change logo to public dir so its "src" will become relative to BASE_PATH

This adds a new test image zipkin-uiproxy which allows us to verify the
ZIPKIN_UI_BASEPATH. This allowed me to notice that we rely on the
basePath having a trailing slash in the index.html.

Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

Here's where I got.

diff --git a/docker/examples/docker-compose-uiproxy.yml b/docker/examples/docker-compose-uiproxy.yml
index a01658436..a94ff1634 100644
--- a/docker/examples/docker-compose-uiproxy.yml
+++ b/docker/examples/docker-compose-uiproxy.yml
@@ -13,7 +13,7 @@ version: '2.4'
 
 services:
   zipkin-uiproxy:
-    image: ghcr.io/openzipkin/zipkin-uiproxy:${TAG:-latest}
+    image: openzipkin/zipkin-uiproxy:test
     container_name: zipkin-uiproxy
     environment:
       # This allows hitting the UI on the host by http://localhost/admin/zipkin
diff --git a/docker/examples/docker-compose.yml b/docker/examples/docker-compose.yml
index 1210bb8d5..b37c6fed0 100644
--- a/docker/examples/docker-compose.yml
+++ b/docker/examples/docker-compose.yml
@@ -17,7 +17,7 @@ services:
   # The zipkin process services the UI, and also exposes a POST endpoint that
   # instrumentation can send trace data to.
   zipkin:
-    image: ghcr.io/openzipkin/zipkin-slim:${TAG:-latest}
+    image: openzipkin/zipkin-slim:test
     container_name: zipkin
     # Environment settings are defined here https://github.com/openzipkin/zipkin/blob/master/zipkin-server/README.md#environment-variables
     environment:
diff --git a/zipkin-lens/vite.config.ts b/zipkin-lens/vite.config.ts
index d8b1b994e..2b9b53839 100644
--- a/zipkin-lens/vite.config.ts
+++ b/zipkin-lens/vite.config.ts
@@ -39,6 +39,7 @@ export default defineConfig(():UserConfig => {
       outDir: 'build',
       // use the same path patterns as the original react-scripts lens build
       assetsDir: "static",
+      sourcemap: true,
       rollupOptions: {
         output: {
           assetFileNames({name}):string {
$ ./mvnw -T1C -q --batch-mode -DskipTests --also-make -pl zipkin-server clean install
$ DOCKER_TARGET=zipkin-slim build-bin/docker/docker_build openzipkin/zipkin-slim:test
$ DOCKER_FILE=docker/test-images/zipkin-uiproxy/Dockerfile build-bin/docker/docker_build openzipkin/zipkin-uiproxy:test
$ cd docker/examples
$ docker-compose -f docker-compose.yml -f docker-compose-uiproxy.yml up

then http://localhost/admin/zipkin/ and add a breakpoint in api.tsx you can see at least for API calls the variable is passing through. Maybe some other code is controlling the window location and it isn't aware of the BASE_PATH?

Screenshot 2024-03-06 at 11 53 46

Adrian Cole added 2 commits March 6, 2024 11:58
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

working except the logo is still served from /zipkin not /admin/zipkin..

Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt
Copy link
Member Author

ok I have it working and verified in the proxy-pass image (zipkin-uiproxy)


const App: React.FC = () => {
useTitle('Zipkin');
const baseName = useMemo(() => {
return import.meta.env.DEV
? '/zipkin'
: (import.meta.env.BASE_URL as string);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basically this was undefined, so why the main URLs weren't mapping

@@ -28,7 +28,6 @@ import TraceIdSearch from './TraceIdSearch';
import TraceJsonUploader from './TraceJsonUploader';
import { useUiConfig } from '../UiConfig';
import { darkTheme } from '../../constants/color';
import logoSrc from '../../img/zipkin-logo.png';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't relatlvize, but since the logo will likely never change, I just moved it to public

Signed-off-by: Adrian Cole <[email protected]>
@@ -1,7 +1,7 @@
<!-- simplified version of /zipkin-lens/index.html -->
<html>
<head>
<base href="/zipkin">
<base href="/zipkin/">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trailing slash is important

@codefromthecrypt codefromthecrypt changed the title lens: adds zipkin-uiproxy image to test ZIPKIN_UI_BASEPATH lens: adds zipkin-uiproxy image and fixes test ZIPKIN_UI_BASEPATH Mar 6, 2024
@codefromthecrypt codefromthecrypt changed the title lens: adds zipkin-uiproxy image and fixes test ZIPKIN_UI_BASEPATH lens: adds zipkin-uiproxy image and fixes ZIPKIN_UI_BASEPATH Mar 6, 2024
@codefromthecrypt codefromthecrypt merged commit 2bbc4bb into master Mar 6, 2024
13 checks passed
@codefromthecrypt codefromthecrypt deleted the fix-uiprozy branch March 6, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant