diff --git a/.changeset/prominent-view-trace-action.md b/.changeset/prominent-view-trace-action.md new file mode 100644 index 0000000000..a0369d63de --- /dev/null +++ b/.changeset/prominent-view-trace-action.md @@ -0,0 +1,12 @@ +--- +'@hyperdx/app': patch +--- + +Make the log side panel "View Trace" action more noticeable: it now uses an +outlined (secondary) button with the trace source icon, larger compact size, and +is right-aligned so it stands out from the dimmed metadata row instead of +blending in as subtle inline text. The first time a log with a correlated trace +is opened, a one-time popover points users to the button; it is dismissed by an +explicit acknowledgement ("Got it" or clicking View Trace) and then never shows +again (persisted per browser). It deliberately does not intercept Escape, which +keeps its normal side-panel behavior. diff --git a/.claude/skills/data-source-icons/SKILL.md b/.claude/skills/data-source-icons/SKILL.md new file mode 100644 index 0000000000..608121a3b0 --- /dev/null +++ b/.claude/skills/data-source-icons/SKILL.md @@ -0,0 +1,55 @@ +--- +name: data-source-icons +description: Use the correct Tabler icon for each HyperDX data source kind (Log, Trace, Session, Metric, PromQL). Use whenever adding or changing an icon that represents a source kind or a signal type — source selectors, side panels, cross-source actions like "View Trace", tabs, badges, breadcrumbs, or anywhere a log/trace/session/metric is depicted. +--- + +# Data source icons + +There is **one canonical icon per source kind**. The source of truth is +`SOURCE_KIND_ICONS` in +`packages/app/src/components/sourceSelectUtils.tsx`. Never invent a different +icon for a source kind (e.g. don't use `IconTimeline` for a trace) — always +match the table below so icons stay consistent across the app. + +## Canonical mapping + +| Source kind (`SourceKind`) | Tabler icon | Meaning | +| -------------------------- | ---------------- | ----------------------------- | +| `Log` | `IconLogs` | Logs | +| `Trace` | `IconConnection` | Traces / spans | +| `Session` | `IconDeviceLaptop` | Session replay / client sessions | +| `Metric` | `IconChartLine` | Metrics | +| `Promql` | `IconChartLine` | PromQL metrics (same as Metric) | + +All imported from `@tabler/icons-react`. + +## Rules + +1. **Prefer the shared map.** When you need a source-kind icon in a context that + already has (or can accept) a `SourceKind`, use `SOURCE_KIND_ICONS[kind]` + from `sourceSelectUtils.tsx` rather than hardcoding an icon component. +2. **If you must hardcode** (e.g. a fixed "View Trace" action that always points + at a trace), import the exact icon from the table above — for a trace that is + `IconConnection`, not `IconTimeline`/`IconRoute`/etc. +3. **Sizing**: the shared map uses `size={16}`. Match the surrounding UI; small + inline buttons/badges commonly use `14`. +4. **Adding a new source kind**: update `SOURCE_KIND_ICONS` first, then update + this table so the two never drift. + +## Examples + +Cross-source "View Trace" action (fixed target → hardcode the trace icon): + +```tsx +import { IconConnection } from '@tabler/icons-react'; + +; +``` + +Dynamic, kind-driven icon (prefer the shared map): + +```tsx +import { SOURCE_KIND_ICONS } from '@/components/sourceSelectUtils'; + +{SOURCE_KIND_ICONS[source.kind]}; +``` diff --git a/knip.json b/knip.json index 23d4e5774c..16cd951d26 100644 --- a/knip.json +++ b/knip.json @@ -26,7 +26,9 @@ }, "packages/common-utils": { "entry": ["src/**/*.ts", "!src/__tests__/**", "!src/**/*.test.*"], - "project": ["src/**/*.ts"] + "project": ["src/**/*.ts"], + "ignoreDependencies": ["@stryker-mutator/core"], + "ignoreBinaries": ["stryker"] }, "packages/hdx-eval": { "project": ["src/**/*.ts"], diff --git a/packages/app/src/components/DBRowSidePanel.tsx b/packages/app/src/components/DBRowSidePanel.tsx index 446978f377..bf90e75a0d 100644 --- a/packages/app/src/components/DBRowSidePanel.tsx +++ b/packages/app/src/components/DBRowSidePanel.tsx @@ -80,6 +80,7 @@ import { import LogLevel from './LogLevel'; import SidePanelBreadcrumbs, { BreadcrumbItem } from './SidePanelBreadcrumbs'; import { SpanLinkData } from './SpanLinksSubpanel'; +import { ViewTraceCalloutButton } from './ViewTraceCalloutButton'; import styles from '@/../styles/LogSidePanel.module.scss'; @@ -440,6 +441,17 @@ export const DBRowSidePanelInner = ({ const severityText: string | undefined = normalizedRow?.['__hdx_severity_text']; + // Owns the View Trace nudge's auto-open latch. Kept here (above the loading + // gate that unmounts the row content on every navigation) so the one-time + // callout doesn't reset and re-open as the user pages between rows. Resets + // when the panel closes and this component unmounts. + const [viewTraceCalloutAutoOpened, setViewTraceCalloutAutoOpened] = + useState(false); + const handleViewTraceCalloutAutoOpen = useCallback( + () => setViewTraceCalloutAutoOpened(true), + [], + ); + // Capture the root event body once for the root breadcrumb label. const [initialMainContent, setInitialMainContent] = useState< string | undefined @@ -931,11 +943,11 @@ export const DBRowSidePanelInner = ({ )} {showLogTraceActions && ( - + /> )} void; + /** + * Whether the nudge has already auto-opened during this panel-open lifecycle. + * Owned by the parent so it survives the per-row loading gate that unmounts + * this button (see `onAutoOpen`). + */ + autoOpened: boolean; + /** Raise the auto-open latch in the parent the first time a trace resolves. */ + onAutoOpen: () => void; +}; + +/** + * The prominent "View trace" action shown in a log side panel, wrapped in a + * one-time nudge popover. + * + * The callout is dismissed only by an explicit acknowledgement — clicking + * "Got it" or the button itself — which persists under its own localStorage key + * (not user settings). It deliberately does not close on stray clicks, drawer + * resizes, or Escape: Escape stays owned by the panel-level hotkey so the + * callout never fights it for the keypress. Because nothing transient is + * stored, the nudge simply reappears the next time the panel opens on an + * eligible log until it is acknowledged. + * + * Auto-open is latched for the panel-open lifecycle (via `autoOpened`/ + * `onAutoOpen`) rather than derived directly from `disabled`, so paging between + * rows doesn't reopen the callout as each row's trace re-resolves. The latch + * lives in the parent because the side panel unmounts this button behind a + * "Loading..." gate while each new row loads, which would reset any state held + * here. + */ +export function ViewTraceCalloutButton({ + disabled, + onView, + autoOpened, + onAutoOpen, +}: ViewTraceCalloutButtonProps) { + const [dismissed, setDismissed] = useLocalStorage( + VIEW_TRACE_CALLOUT_DISMISSED_KEY, + false, + ); + const dismiss = useCallback(() => setDismissed(true), [setDismissed]); + + useEffect(() => { + if (!disabled && !dismissed && !autoOpened) { + onAutoOpen(); + } + }, [disabled, dismissed, autoOpened, onAutoOpen]); + const opened = autoOpened && !dismissed; + + return ( + + + + + + + + Jump to this log's full trace + + + Open the correlated trace in one click. + + + + + + ); +} diff --git a/packages/app/src/components/__tests__/ViewTraceCalloutButton.test.tsx b/packages/app/src/components/__tests__/ViewTraceCalloutButton.test.tsx new file mode 100644 index 0000000000..1606fdd98e --- /dev/null +++ b/packages/app/src/components/__tests__/ViewTraceCalloutButton.test.tsx @@ -0,0 +1,143 @@ +import { ReactNode, useState } from 'react'; +import { MantineProvider } from '@mantine/core'; +import { + fireEvent, + render, + screen, + waitForElementToBeRemoved, +} from '@testing-library/react'; + +import { VIEW_TRACE_CALLOUT_DISMISSED_KEY } from '@/components/viewTraceCallout'; +import { ViewTraceCalloutButton } from '@/components/ViewTraceCalloutButton'; + +// A provider that is reapplied on rerender, so prop transitions (row +// navigation) can be exercised across rerenders. +function MantineWrapper({ children }: { children: ReactNode }) { + return {children}; +} + +const noop = () => undefined; + +// Mirrors the real owner (DBRowSidePanelInner): it holds the auto-open latch and +// gates the button behind a "Loading..." state while a row resolves, unmounting +// the button exactly as the side panel does. The latch outlives that gate. +function PanelHarness({ + disabled = false, + loading = false, + onView = noop, +}: { + disabled?: boolean; + loading?: boolean; + onView?: () => void; +}) { + const [autoOpened, setAutoOpened] = useState(false); + + if (loading) { + return
Loading...
; + } + + return ( + setAutoOpened(true)} + /> + ); +} + +describe('ViewTraceCalloutButton', () => { + beforeEach(() => { + window.localStorage.clear(); + }); + + it('auto-opens the one-time callout when enabled and not yet dismissed', async () => { + render(, { wrapper: MantineWrapper }); + + expect(screen.getByTestId('side-panel-view-trace')).toBeEnabled(); + // Auto-open is raised via an effect, so the dropdown mounts asynchronously. + expect(await screen.findByTestId('view-trace-callout')).toBeInTheDocument(); + expect(screen.getByText('Got it')).toBeInTheDocument(); + }); + + it('does not open the callout while the trace is unresolved (disabled)', () => { + render(, { wrapper: MantineWrapper }); + + expect(screen.getByTestId('side-panel-view-trace')).toBeDisabled(); + expect(screen.queryByTestId('view-trace-callout')).not.toBeInTheDocument(); + }); + + it('does not show the callout once it has been dismissed previously', () => { + window.localStorage.setItem( + VIEW_TRACE_CALLOUT_DISMISSED_KEY, + JSON.stringify(true), + ); + + render(, { wrapper: MantineWrapper }); + + // Button still available, but the nudge no longer appears. + expect(screen.getByTestId('side-panel-view-trace')).toBeInTheDocument(); + expect(screen.queryByTestId('view-trace-callout')).not.toBeInTheDocument(); + }); + + it('persists dismissal and navigates when the View Trace button is clicked', async () => { + const onView = jest.fn(); + render(, { wrapper: MantineWrapper }); + + fireEvent.click(await screen.findByTestId('side-panel-view-trace')); + + expect(onView).toHaveBeenCalledTimes(1); + expect(window.localStorage.getItem(VIEW_TRACE_CALLOUT_DISMISSED_KEY)).toBe( + JSON.stringify(true), + ); + // The popover plays an exit transition before unmounting. + await waitForElementToBeRemoved(() => + screen.queryByTestId('view-trace-callout'), + ); + }); + + it('persists dismissal without navigating when "Got it" is clicked', async () => { + const onView = jest.fn(); + render(, { wrapper: MantineWrapper }); + + fireEvent.click(await screen.findByText('Got it')); + + expect(onView).not.toHaveBeenCalled(); + expect(window.localStorage.getItem(VIEW_TRACE_CALLOUT_DISMISSED_KEY)).toBe( + JSON.stringify(true), + ); + await waitForElementToBeRemoved(() => + screen.queryByTestId('view-trace-callout'), + ); + }); + + it('opens the callout once the trace resolves (disabled → enabled)', async () => { + const { rerender } = render(, { + wrapper: MantineWrapper, + }); + + expect(screen.queryByTestId('view-trace-callout')).not.toBeInTheDocument(); + + rerender(); + + expect(await screen.findByTestId('view-trace-callout')).toBeInTheDocument(); + }); + + it('stays latched across the per-row loading gate that unmounts the button', async () => { + // Paging to another row drops the panel into a "Loading..." state that + // unmounts the button. Because the latch is owned by the parent, the nudge + // must come back already open (synchronously, no re-run of the auto-open + // effect) rather than reappearing from scratch on every row. + const { rerender } = render(, { wrapper: MantineWrapper }); + + expect(await screen.findByTestId('view-trace-callout')).toBeInTheDocument(); + + // Next row starts loading: the button (and its popover) unmount. + rerender(); + expect(screen.queryByTestId('view-trace-callout')).not.toBeInTheDocument(); + + // Row settles: the button remounts and the callout is open immediately. + rerender(); + expect(screen.getByTestId('view-trace-callout')).toBeInTheDocument(); + }); +}); diff --git a/packages/app/src/components/viewTraceCallout.ts b/packages/app/src/components/viewTraceCallout.ts new file mode 100644 index 0000000000..45cc459105 --- /dev/null +++ b/packages/app/src/components/viewTraceCallout.ts @@ -0,0 +1,9 @@ +/** + * localStorage key for the one-time "View trace" button callout dismissal. + * + * Kept in this dependency-free module so it can be shared by the React + * component and the Playwright fixture (which seeds it) without either copy + * drifting from the other. + */ +export const VIEW_TRACE_CALLOUT_DISMISSED_KEY = + 'hdx-view-trace-callout-dismissed'; diff --git a/packages/app/tests/e2e/utils/base-test.ts b/packages/app/tests/e2e/utils/base-test.ts index 8844d964f9..3537b976f8 100644 --- a/packages/app/tests/e2e/utils/base-test.ts +++ b/packages/app/tests/e2e/utils/base-test.ts @@ -2,6 +2,8 @@ import fs from 'fs'; import path from 'path'; import { expect, test as base } from '@playwright/test'; +import { VIEW_TRACE_CALLOUT_DISMISSED_KEY } from '../../../src/components/viewTraceCallout'; + // Single source of truth: e2e-fixtures.json (connections/sources). API gets them via run-api-with-fixtures.js. const E2E_FIXTURES_PATH = path.join(__dirname, '../fixtures/e2e-fixtures.json'); function loadE2EFixtures(): { connections: unknown[]; sources: unknown[] } { @@ -28,7 +30,7 @@ export const test = base.extend({ // e2e-fixtures.json so local mode uses the same data as full-stack. await page.addInitScript( (arg: unknown[]) => { - const [connections, sources] = arg; + const [connections, sources, calloutDismissedKey] = arg; window.localStorage.setItem('TanstackQueryDevtools.open', 'false'); window.sessionStorage.setItem( 'connections', @@ -38,8 +40,19 @@ export const test = base.extend({ 'hdx-local-source', JSON.stringify(sources), ); + // Suppress the one-time "View trace" callout so its auto-opening + // popover never interferes with side-panel interactions. useLocalStorage + // JSON-encodes values, so store the boolean as JSON. + window.localStorage.setItem( + String(calloutDismissedKey), + JSON.stringify(true), + ); }, - [e2eFixtures.connections, e2eFixtures.sources], + [ + e2eFixtures.connections, + e2eFixtures.sources, + VIEW_TRACE_CALLOUT_DISMISSED_KEY, + ], ); await fn(page); },