lens: fixes BASE_PATH handling and re-aligns to old resource paths (#…

…3722) The last commit moved things forward, but it broke how we used the BASE_PATH variable. This reverts to the old logic and fixes up the build so it behaved like the last one. Along the way, I discovered some tests were disabled, and as able to fix them except one. Signed-off-by: Adrian Cole <[email protected]>
openzipkin · Feb 12, 2024 · 39e9f84 · 39e9f84
1 parent 2ada407
commit 39e9f84
Show file tree

Hide file tree

Showing 30 changed files with 160 additions and 109 deletions.
diff --git a/pom.xml b/pom.xml
@@ -533,6 +533,7 @@
             <exclude>**/.idea/**</exclude>
             <exclude>**/node_modules/**</exclude>
             <exclude>**/build/**</exclude>
+            <exclude>**/dist/**</exclude>
             <exclude>**/coverage/**</exclude>
             <exclude>**/.babelrc</exclude>
             <exclude>**/.bowerrc</exclude>

diff --git a/zipkin-lens/.env.development b/zipkin-lens/.env.development
diff --git a/zipkin-lens/.env.production b/zipkin-lens/.env.production
diff --git a/zipkin-lens/.env.test b/zipkin-lens/.env.test
diff --git a/zipkin-lens/.gitignore b/zipkin-lens/.gitignore
@@ -10,6 +10,7 @@
 
 # production
 /build
+/dist
 
 # misc
 .DS_Store

diff --git a/zipkin-lens/.npmrc b/zipkin-lens/.npmrc
@@ -0,0 +1,2 @@
+# This is needed while dependencies rely on different versions of react 
+legacy-peer-deps=true
diff --git a/zipkin-lens/README.md b/zipkin-lens/README.md
@@ -34,6 +34,18 @@ To get a coverage report as well, run `npm test -- --coverage`.
 Builds the app for production to the `build` folder.<br />
 It correctly bundles React in production mode and optimizes the build for the best performance.
 
+## Build tips
+
+### Use the production node version
+
+The production UI is built with Maven. To use the same version, issue this command:
+
+```bash
+nvm use $(../mvnw help:evaluate -Dexpression=node.version -q -DforceStdout)
+```
+
+Now, it is less likely a pull request will fail when `npm test` succeeds locally.
+
 ## Localization
 
 We use [LinguiJS](https://lingui.js.org/) for localization of the UI. Translations for strings are

diff --git a/zipkin-lens/index.html b/zipkin-lens/index.html
@@ -16,6 +16,7 @@
 <!DOCTYPE html>
 <html>
   <head>
+    <base href="/zipkin" >
     <meta charset="utf-8" />
     <link rel="icon" href="/favicon.ico" />
   </head>

diff --git a/zipkin-lens/package-lock.json b/zipkin-lens/package-lock.json
diff --git a/zipkin-lens/pom.xml b/zipkin-lens/pom.xml
@@ -116,7 +116,7 @@
               <goal>npm</goal>
             </goals>
             <configuration>
-              <arguments>install --legacy-peer-deps</arguments>
+              <arguments>install</arguments>
             </configuration>
           </execution>
           <execution>

diff --git a/zipkin-lens/src/components/App/App.tsx b/zipkin-lens/src/components/App/App.tsx
@@ -37,7 +37,7 @@ const App: React.FC = () => {
   const baseName = useMemo(() => {
     return import.meta.env.DEV
       ? '/zipkin'
-      : (import.meta.env.BASE_PATH as string);
+      : (import.meta.env.BASE_URL as string);
   }, []);
 
   return (

diff --git a/zipkin-lens/src/components/App/LanguageSelector.tsx b/zipkin-lens/src/components/App/LanguageSelector.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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

diff --git a/zipkin-lens/src/components/App/Layout.tsx b/zipkin-lens/src/components/App/Layout.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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

diff --git a/zipkin-lens/src/components/App/TraceIdSearch.tsx b/zipkin-lens/src/components/App/TraceIdSearch.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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
@@ -11,7 +11,6 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 import { TextField } from '@material-ui/core';
 import React, { useCallback, useState } from 'react';
 import { useHistory } from 'react-router-dom';

diff --git a/zipkin-lens/src/components/DiscoverPage/TraceSummaryTable.tsx b/zipkin-lens/src/components/DiscoverPage/TraceSummaryTable.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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
@@ -11,7 +11,6 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 import {
   Table,
   TableBody,

diff --git a/zipkin-lens/src/components/TracePage/Header/Header.tsx b/zipkin-lens/src/components/TracePage/Header/Header.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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
@@ -11,7 +11,6 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 import { Box, Button, makeStyles, Typography } from '@material-ui/core';
 import { List as ListIcon } from '@material-ui/icons';
 import React from 'react';

diff --git a/zipkin-lens/src/components/TracePage/Header/HeaderMenu.tsx b/zipkin-lens/src/components/TracePage/Header/HeaderMenu.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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
@@ -11,7 +11,6 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 import { Button, makeStyles, Menu, MenuItem } from '@material-ui/core';
 import { Menu as MenuIcon } from '@material-ui/icons';
 import React, { useCallback } from 'react';

diff --git a/zipkin-lens/src/components/TracePage/SpanDetailDrawer/SpanDetailDrawer.tsx b/zipkin-lens/src/components/TracePage/SpanDetailDrawer/SpanDetailDrawer.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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
@@ -11,7 +11,6 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 import { Box, Divider, Grid, makeStyles, Typography } from '@material-ui/core';
 import React from 'react';
 import { useTranslation } from 'react-i18next';

diff --git a/zipkin-lens/src/components/TracePage/Timeline/Timeline.tsx b/zipkin-lens/src/components/TracePage/Timeline/Timeline.tsx
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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
@@ -11,7 +11,6 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 import { Box, makeStyles } from '@material-ui/core';
 import React, { useCallback, useEffect, useRef } from 'react';
 import AutoSizer, { Size } from 'react-virtualized-auto-sizer';

diff --git a/...rc/components/UiConfig/UiConfig.notest.js → ...src/components/UiConfig/UiConfig.test.jsx b/...rc/components/UiConfig/UiConfig.notest.js → ...src/components/UiConfig/UiConfig.test.jsx
@@ -13,65 +13,60 @@
  */
 import { render, screen } from '@testing-library/react';
 import fetchMock from 'fetch-mock';
-import { afterEach, beforeEach, it, describe, expect } from 'vitest';
+import { afterEach, it, describe, expect, vi } from 'vitest';
 import React, { Suspense } from 'react';
 
-import {
-  defaultConfig,
-  UiConfigConsumer,
-  UiConfig as RawUIConfig,
-} from './UiConfig';
-import { UI_CONFIG } from '../../constants/api';
-
 afterEach(() => {
   fetchMock.restore();
+  vi.resetModules();
 });
 
-beforeEach(() => {
-  // We fetch the resource on module initialization for performance, but want to do that in every
-  // test.
-});
-
-const UiConfig = () => {
-  return (
+const renderUiConfig = async () => {
+  const { UiConfigConsumer, UiConfig } = await import('./UiConfig');
+  render(
     <Suspense fallback="Suspended">
-      <RawUIConfig>
+      <UiConfig>
         <UiConfigConsumer>
           {(value) => <div>{JSON.stringify(value)}</div>}
         </UiConfigConsumer>
-      </RawUIConfig>
-    </Suspense>
+      </UiConfig>
+    </Suspense>,
   );
 };
 
 describe('<UiConfig />', () => {
   it('fetches config and suspends', async () => {
     const configPromise = new Promise(() => undefined);
+    const { UI_CONFIG } = await import('../../constants/api');
     fetchMock.once(UI_CONFIG, configPromise, { overwriteRoutes: true });
 
-    render(<UiConfig />);
+    await renderUiConfig();
     expect(screen.getAllByText('Suspended')).length(1);
 
     fetchMock.called(UI_CONFIG);
   });
 
-  it('provides config when resolved', async () => {
+  it('provides config when resolved', (context) => async () => {
     const config = { defaultLookback: 100 };
+    const { defaultConfig } = await import('./UiConfig');
     Object.keys(defaultConfig).forEach((key) => {
       config[key] = config[key] || defaultConfig[key];
     });
-
+    const { UI_CONFIG } = await import('../../constants/api');
     fetchMock.once(UI_CONFIG, config, { overwriteRoutes: true });
 
-    render(<UiConfig />);
-    expect(screen.getAllByText('Suspended').length).toBe(1);
+    // TODO: adrian needs help, as this broke when porting to vitest
+    context.skip();
+
+    const { rerender } = await renderUiConfig();
+    expect(screen.getAllByText('Suspended')).length(1);
 
-    /*// We need to get off the processing loop to allow the promise to complete and resolve the
+    // We need to get off the processing loop to allow the promise to complete and resolve the
     // config.
     await new Promise((resolve) => setTimeout(resolve, 1));
 
-    rerender(<UiConfig />);
-    expect(screen.getByText(JSON.stringify(config))).toBeInTheDocument();*/
+    rerender(); // was rerender(<UiConfig />);
+    expect(screen.getByText(JSON.stringify(config))).toBeInTheDocument();
 
     fetchMock.called(UI_CONFIG);
   });

diff --git a/zipkin-lens/src/constants/api.test.tsx b/zipkin-lens/src/constants/api.test.tsx
@@ -0,0 +1,39 @@
+/*
+ * 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.
+ */
+
+/* eslint-disable global-require */
+
+import { afterEach, it, describe, expect, vi } from 'vitest';
+
+// To test BASE_PATH we can't import it above, as it would return a constant
+// value for all tests. Instead, we use `await import` later.
+describe('BASE_PATH', () => {
+  afterEach(() => {
+    vi.resetModules();
+  });
+
+  it('defaults to /zipkin with no base tag', async () => {
+    const { BASE_PATH } = await import('./api');
+    expect(BASE_PATH).toEqual('/zipkin');
+  });
+
+  it('is set to base tag when present', async () => {
+    const base = document.createElement('base');
+    base.setAttribute('href', '/coolzipkin/');
+    document.head.append(base);
+
+    const { BASE_PATH } = await import('./api');
+    expect(BASE_PATH).toEqual('/coolzipkin');
+  });
+});
diff --git a/zipkin-lens/src/constants/api.ts b/zipkin-lens/src/constants/api.ts
@@ -11,8 +11,17 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-export const BASE_PATH = import.meta.env.BASE_PATH;
-export const ZIPKIN_API = `${import.meta.env.API_BASE}/api/v2`;
+const extractBasePath = () => {
+  const base = document.getElementsByTagName('base');
+  if (base.length === 0) {
+    return '/zipkin';
+  }
+  return base[0]?.getAttribute('href')?.replace(/\/+$/, '');
+};
+
+export const BASE_PATH = extractBasePath();
+
+export const ZIPKIN_API = `${BASE_PATH}/api/v2`;
 export const UI_CONFIG = `${BASE_PATH}/config.json`;
 export const SERVICES = `${ZIPKIN_API}/services`;
 export const REMOTE_SERVICES = `${ZIPKIN_API}/remoteServices`;

diff --git a/zipkin-lens/src/setupProxy.js b/zipkin-lens/src/setupProxy.js
diff --git a/zipkin-lens/src/setupTests.ts b/zipkin-lens/src/setupTests.ts
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015-2023 The OpenZipkin Authors
+ * 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
@@ -11,7 +11,6 @@
  * or implied. See the License for the specific language governing permissions and limitations under
  * the License.
  */
-
 import '@testing-library/jest-dom';
 import fetchMock from 'fetch-mock';