feat(apm): editor latency gutter markers + popover#2839
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
React Doctor found 2 issues in 1 file · 2 warnings. 2 warnings
Reviewed by React Doctor for commit |
|
Reviews (1): Last reviewed commit: "feat(apm): editor latency gutter markers..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it("signs and rounds a meaningful delta", () => { | ||
| expect(formatPercentDelta(186.2)).toBe("+186%"); | ||
| expect(formatPercentDelta(-5.2)).toBe("-5%"); | ||
| expect(formatPercentDelta(1.43)).toBe("+1%"); | ||
| expect(formatPercentDelta(-0.9)).toBe("-1%"); | ||
| }); | ||
|
|
||
| it("hides sub-1% noise rather than rendering a meaningless +0%/-0%", () => { | ||
| expect(formatPercentDelta(0)).toBeNull(); | ||
| expect(formatPercentDelta(0.4)).toBeNull(); | ||
| expect(formatPercentDelta(-0.4)).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
it blocks each bundle multiple independent input/expected pairs as sequential assertions. Per the team's preference, these should use it.each so each pair gets its own test row — a single assertion failure won't shadow the rest, and the test name itself documents the input.
| it("signs and rounds a meaningful delta", () => { | |
| expect(formatPercentDelta(186.2)).toBe("+186%"); | |
| expect(formatPercentDelta(-5.2)).toBe("-5%"); | |
| expect(formatPercentDelta(1.43)).toBe("+1%"); | |
| expect(formatPercentDelta(-0.9)).toBe("-1%"); | |
| }); | |
| it("hides sub-1% noise rather than rendering a meaningless +0%/-0%", () => { | |
| expect(formatPercentDelta(0)).toBeNull(); | |
| expect(formatPercentDelta(0.4)).toBeNull(); | |
| expect(formatPercentDelta(-0.4)).toBeNull(); | |
| }); | |
| it.each([ | |
| [186.2, "+186%"], | |
| [-5.2, "-5%"], | |
| [1.43, "+1%"], | |
| [-0.9, "-1%"], | |
| ] as const)("signs and rounds %s → %s", (input, expected) => { | |
| expect(formatPercentDelta(input)).toBe(expected); | |
| }); | |
| it.each([0, 0.4, -0.4])( | |
| "hides sub-1%% noise: formatPercentDelta(%s) → null", | |
| (input) => { | |
| expect(formatPercentDelta(input)).toBeNull(); | |
| }, | |
| ); |
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| { filePath }, | ||
| { | ||
| enabled, | ||
| staleTime: Number.POSITIVE_INFINITY, |
There was a problem hiding this comment.
staleTime: Number.POSITIVE_INFINITY caches the APM data for the entire session with no automatic refresh. After a deployment or any production change, engineers will continue seeing the old latency numbers with no indication they are stale. A bounded stale time (e.g. 5 minutes) lets React Query silently background-refresh without causing visible loading flickers.
| staleTime: Number.POSITIVE_INFINITY, | |
| staleTime: 5 * 60 * 1000, // 5 minutes – APM data is cheap to refresh and should not go stale silently |
jonmcwest
force-pushed
the
06-22-feat_apm_editor_apm_enrichment_service_trpc_route
branch
from
214b340 to
df9dcd6
Compare
jonmcwest
force-pushed
the
06-22-feat_apm_editor_latency_gutter_markers_popover
branch
from
0cfe05e to
8ffd7e0
Compare
1 participant
Problem
When reviewing code in the editor, engineers have no visibility into how that code performs in production. Switching to PostHog tracing to correlate spans with source lines requires manual cross-referencing. This adds inline APM (Application Performance Monitoring) enrichment to the code editor, surfacing real production latency and span data directly in the gutter alongside the relevant source lines.
Changes
APM eligibility and marker building (
@posthog/core)isApmEnrichmentEligibleto gate enrichment by file extension, delegating to the sharedapmLangForFileso the editor and agent paths share one supported-language list.buildApmLineMarkersto convertSerializedApmEnrichmentstats into per-line gutter marker objects, each carrying the underlyingSpanLineStatand a single-line tooltip summary (p95, p50, span count, error count).formatPercentDeltafrom@posthog/sharedthroughenrichmentPresentersso the editor popover's existing import path stays stable while the agent path can also use it without importing@posthog/core.CodeMirror extension (
@posthog/ui)postHogApmEnrichmentExtension, a CodeMirror state field + gutter that renders a fixed-colour purple presence marker on every instrumented line. Clicking a marker opens the APM popover. Markers re-anchor on doc changes to stay correct if the view becomes editable.APM popover
ApmEnrichmentPopover, a fixed-position portal card showing p50/p95/p99 latency, span count, error rate, and percentage deltas vs. the prior window. Deltas are colour-coded (red when latency increases, green when it decreases). A "View in PostHog →" link deep-links to the tracing explorer. The popover closes on outside click or Escape.apmPopoverStore(Zustand) to manage open/close state and the anchor rect independently of the existing enrichment popover store.Data fetching
useFileApmEnrichmenthook that callstrpc.apmEnrichment.enrichFile, gated behind theAPM_ENRICHMENT_FLAGfeature flag (auto-enabled in dev) and file-type eligibility. Returnsundefinedwhen the gutter extension should be skipped entirely, andnullwhile data is loading.apmEnrichmentintoCodeEditorPanelandCodeMirrorEditor. The APM popover is dismissed automatically when switching files.How did you test this?
isApmEnrichmentEligiblecovering Rust, Go, Python, TypeScript, Java source files, and non-source files (Markdown, JSON, CSS, PNG).buildApmLineMarkerscovering null enrichment, marker-per-line production, stat pass-through, and tooltip summary formatting.formatPercentDeltacovering null/undefined input, signed rounding, and sub-1% noise suppression.Automatic notifications