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

fix: Fix UI issues when we get a bare span envelope #726

Merged
merged 1 commit into from
Mar 5, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/nervous-ties-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@spotlightjs/spotlight': patch
'@spotlightjs/electron': patch
'@spotlightjs/overlay': patch
'@spotlightjs/astro': patch
---

Fix UI issues when we get a bare span envelope
3 changes: 3 additions & 0 deletions packages/overlay/_fixtures/envelope_with_only_span.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{"sent_at":"2025-03-04T21:23:54.940Z","trace":{"environment":"development","release":"spotlight@undefined","public_key":"51bcd92dba1128934afd1c5726c84442","trace_id":"b99011b43059c992d5dab0b8281f519b","sample_rate":"1","transaction":"Spotlight CLI","sampled":"true"}}
{"type":"span"}
{"data":{"sentry.origin":"auto.http.browser.inp","sentry.op":"ui.interaction.click","sentry.source":"custom","release":"spotlight@2.12.1","environment":"production","replay_id":"530121597fe04f93a4ebf23b55748e00","transaction":"/errors","user_agent.original":"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/133.0.0.0 Safari/537.36 Edg/133.0.0.0","sentry.exclusive_time":5576,"sentry.sample_rate":1},"description":"body > div#sentry-spotlight-root","op":"ui.interaction.click","parent_span_id":"8044e7503e635f42","span_id":"ae2cb2899baf38a4","start_timestamp":1741123427.9582,"timestamp":1741123433.5342,"trace_id":"b99011b43059c992d5dab0b8281f519b","origin":"auto.http.browser.inp","exclusive_time":5576,"measurements":{"inp":{"value":5576,"unit":"millisecond"}},"is_segment":true,"segment_id":"ae2cb2899baf38a4"}
Empty file modified packages/overlay/_fixtures/send_to_sidecar.cjs
100644 → 100755
Empty file.
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@
import Badge from '~/ui/Badge';
import CardList from '../../../../components/CardList';
import TimeSince from '../../../../components/TimeSince';
import { isErrorEvent } from '../../data/sentryDataCache';
import { useSentryEvents } from '../../data/useSentryEvents';
import { useSentryHelpers } from '../../data/useSentryHelpers';
import { truncateId } from '../../utils/text';
@@ -16,7 +17,7 @@
const helpers = useSentryHelpers();
const context = useSpotlightContext();

const matchingEvents = events.filter(e => e.type !== 'transaction' && e.type !== 'profile');
const matchingEvents = events.filter(isErrorEvent);

Check warning on line 20 in packages/overlay/src/integrations/sentry/components/events/EventList.tsx

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/events/EventList.tsx#L20

Added line #L20 was not covered by tests

const [showAll, setShowAll] = useState(!context.experiments['sentry:focus-local-events']);
const filteredEvents = showAll
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Link, Navigate, Route, Routes, useParams } from 'react-router-dom';
import { createTab } from '~/integrations/sentry/utils/tabs';
import Tabs from '../../../../../../components/Tabs';
import { default as dataCache } from '../../../../data/sentryDataCache';
import { default as dataCache, isErrorEvent } from '../../../../data/sentryDataCache';
import { useSentryEvents } from '../../../../data/useSentryEvents';
import { useSentryHelpers } from '../../../../data/useSentryHelpers';
import EventContexts from '../../../events/EventContexts';
@@ -34,8 +34,7 @@

const errorCount = events.filter(
e =>
e.type !== 'transaction' &&
e.type !== 'profile' &&
isErrorEvent(e) &&

Check warning on line 37 in packages/overlay/src/integrations/sentry/components/explore/traces/TraceDetails/index.tsx

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/explore/traces/TraceDetails/index.tsx#L37

Added line #L37 was not covered by tests
(e.contexts?.trace?.trace_id ? helpers.isLocalToSession(e.contexts?.trace?.trace_id) : null) !== false,
).length;

Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { useState, type ReactNode } from 'react';
import { type ReactNode, useState } from 'react';
import { Link } from 'react-router-dom';
import { format as formatSQL } from 'sql-formatter';
import JsonViewer from '../../../../../../components/JsonViewer';
import SidePanel, { SidePanelHeader } from '../../../../../../ui/SidePanel';
import { DB_SPAN_REGEX } from '../../../../constants';
import dataCache from '../../../../data/sentryDataCache';
import dataCache, { isErrorEvent } from '../../../../data/sentryDataCache';
import type { SentryErrorEvent, Span, TraceContext } from '../../../../types';
import { formatBytes } from '../../../../utils/bytes';
import { getFormattedDuration } from '../../../../utils/duration';
@@ -90,7 +90,7 @@

const spanDuration = span.timestamp - span.start_timestamp;

const errors = dataCache.getEventsByTrace(span.trace_id).filter(e => e.type !== 'transaction' && 'exception' in e);
const errors = span.trace_id ? dataCache.getEventsByTrace(span.trace_id).filter(isErrorEvent) : [];

Check warning on line 93 in packages/overlay/src/integrations/sentry/components/explore/traces/spans/SpanDetails.tsx

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/explore/traces/spans/SpanDetails.tsx#L93

Added line #L93 was not covered by tests

return (
<SidePanel backto={`/explore/traces/${span.trace_id}`}>
Original file line number Diff line number Diff line change
@@ -15,8 +15,10 @@
type QuerySummarySortTypes = (typeof QUERY_SUMMARY_SORT_KEYS)[keyof typeof QUERY_SUMMARY_SORT_KEYS];
const COMPARATORS: Record<QuerySummarySortTypes, SpanInfoComparator> = {
[QUERY_SUMMARY_SORT_KEYS.foundIn]: (a, b) => {
if (a.trace_id < b.trace_id) return -1;
if (a.trace_id > b.trace_id) return 1;
const aTrace = a.trace_id || '';
const bTrace = b.trace_id || '';
if (aTrace < bTrace) return -1;
if (aTrace > bTrace) return 1;

Check warning on line 21 in packages/overlay/src/integrations/sentry/components/insights/QuerySummary.tsx

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/insights/QuerySummary.tsx#L18-L21

Added lines #L18 - L21 were not covered by tests
return 0;
},
[QUERY_SUMMARY_SORT_KEYS.spanId]: (a, b) => {
4 changes: 2 additions & 2 deletions packages/overlay/src/integrations/sentry/data/profiles.ts
Original file line number Diff line number Diff line change
@@ -126,7 +126,7 @@
const frame = profile.frames[currentStack[frameIdxIdx]];
// XXX: We may wanna skip frames that doesn't have `in_app` set to true
// that said it's better to have this as a dynamic filter
const spanFromFrame = {
const spanFromFrame: Span = {

Check warning on line 129 in packages/overlay/src/integrations/sentry/data/profiles.ts

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/profiles.ts#L129

Added line #L129 was not covered by tests
span_id: generateUuidv4(),
parent_span_id: currentSpan.span_id,
...commonAttributes,
@@ -168,7 +168,7 @@
return;
}
if (!profile) {
profile = sentryDataCache.getProfileByTraceId(trace.trace_id);
profile = trace.trace_id ? sentryDataCache.getProfileByTraceId(trace.trace_id) : undefined;
if (!profile) {
log(`Profile not found for trace ${trace.trace_id}`);
return;
37 changes: 29 additions & 8 deletions packages/overlay/src/integrations/sentry/data/sentryDataCache.ts
Original file line number Diff line number Diff line change
@@ -32,7 +32,8 @@
// 'event' really is 'error' here but \(〇_o)/

const ERROR_EVENT_TYPES = new Set(['event', 'error']);
const TRACE_EVENT_TYPES = new Set(['transaction', 'span']);
// Enable 'span' type later on. See https://github.com/getsentry/spotlight/issues/721
const TRACE_EVENT_TYPES = new Set(['transaction'/*, 'span'*/]);
const PROFILE_EVENT_TYPES = new Set(['profile']);
const SUPPORTED_EVENT_TYPES = new Set([...ERROR_EVENT_TYPES, ...TRACE_EVENT_TYPES, ...PROFILE_EVENT_TYPES]);

@@ -128,9 +129,18 @@
});
}

const traceContext = header.trace;

for (const [itemHeader, itemData] of items) {
if (SUPPORTED_EVENT_TYPES.has(itemHeader.type)) {
(itemData as SentryEvent).platform = sdkToPlatform(sdk.name);
const item = itemData as SentryEvent;
item.platform = sdkToPlatform(sdk.name);
if (traceContext) {
if (!item.contexts) {
item.contexts = {};
}

Check warning on line 141 in packages/overlay/src/integrations/sentry/data/sentryDataCache.ts

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/sentryDataCache.ts#L140-L141

Added lines #L140 - L141 were not covered by tests
item.contexts.trace ??= traceContext;
}
// The below is an async function but we really don't need to wait for that
this.pushEvent(itemData as SentryEvent);
}
@@ -164,11 +174,12 @@

this.events.push(event);

if (traceCtx) {
if (traceCtx?.trace_id) {
const existingTrace = this.tracesById.get(traceCtx.trace_id);
const startTs = event.start_timestamp ? event.start_timestamp : new Date().getTime();
const trace = existingTrace ?? {
...traceCtx,
trace_id: traceCtx.trace_id,
spans: new Map(),
spanTree: [] as Span[],
transactions: [] as SentryTransactionEvent[],
@@ -189,8 +200,18 @@
// XXX: we're trusting timestamps, which may not be as reliable as we'd like
const spanMap: Map<string, Span> = new Map();
for (const txn of trace.transactions) {
spanMap.set(txn.contexts.trace.span_id, {
...txn.contexts.trace,
const trace = txn.contexts.trace;
if (!trace || !trace.span_id || !trace.trace_id) {
continue;
}

Check warning on line 206 in packages/overlay/src/integrations/sentry/data/sentryDataCache.ts

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/sentryDataCache.ts#L205-L206

Added lines #L205 - L206 were not covered by tests

spanMap.set(trace.span_id, {
...trace,
// TypeScript is not smart enough to compose the assertion above
// with the spread syntax above, hence the need for these explicit
// `span_id` and `trace_id` assignments
span_id: trace.span_id,
trace_id: trace.trace_id,
tags: txn?.tags,
start_timestamp: txn.start_timestamp,
timestamp: txn.timestamp,
@@ -400,14 +421,14 @@

export default new SentryDataCache();

function isErrorEvent(event: SentryEvent): event is SentryErrorEvent {
export function isErrorEvent(event: SentryEvent): event is SentryErrorEvent {
return (!event.type || ERROR_EVENT_TYPES.has(event.type)) && Boolean((event as SentryErrorEvent).exception);
}

function isProfileEvent(event: SentryEvent): event is SentryProfileV1Event {
export function isProfileEvent(event: SentryEvent): event is SentryProfileV1Event {
return !!event.type && PROFILE_EVENT_TYPES.has(event.type) && (event as SentryProfileV1Event).version === '1';
}

function isTraceEvent(event: SentryEvent): event is SentryTransactionEvent {
export function isTraceEvent(event: SentryEvent): event is SentryTransactionEvent {
return !!event.type && TRACE_EVENT_TYPES.has(event.type);
}
7 changes: 3 additions & 4 deletions packages/overlay/src/integrations/sentry/index.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { Client, Envelope, EnvelopeItem } from '@sentry/types';
import type { Client, Envelope, EnvelopeItem } from '@sentry/core';
import { removeURLSuffix } from '~/lib/removeURLSuffix';
import { off, on } from '../../lib/eventTarget';
import { log, warn } from '../../lib/logger';
import type { Integration, RawEventContext } from '../integration';
import sentryDataCache from './data/sentryDataCache';
import { isErrorEvent, default as sentryDataCache } from './data/sentryDataCache';
import { spotlightIntegration } from './sentry-integration';
import ErrorsTab from './tabs/ErrorsTab';
import ExploreTab from './tabs/ExploreTab';
@@ -65,8 +65,7 @@
.getEvents()
.filter(
e =>
e.type !== 'transaction' &&
e.type !== 'profile' &&
isErrorEvent(e) &&

Check warning on line 68 in packages/overlay/src/integrations/sentry/index.ts

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/index.ts#L68

Added line #L68 was not covered by tests
(e.contexts?.trace?.trace_id ? sentryDataCache.isTraceLocal(e.contexts?.trace?.trace_id) : null) !== false,
).length;

27 changes: 15 additions & 12 deletions packages/overlay/src/integrations/sentry/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Measurements } from '@sentry/core';
import type { EventEnvelopeHeaders, Measurements } from '@sentry/core';

export type FrameVars = {
[key: string]: string;
@@ -70,14 +70,16 @@ type CommonEventAttrs = {
measurements?: Measurements;
};

export type TraceContext = {
trace_id: string;
span_id: string;
parent_span_id?: string | null;
op: string;
description?: string | null;
status: 'ok' | string;
data?: Context;
// Note: For some reason the `sentry/core` module doesn't have these additional properties
// in `EventEnvelopeHeaders['trace']` but they are present in the actual events.
// Follow up?
export type TraceContext = EventEnvelopeHeaders['trace'] & {
span_id?: string;
status?: 'ok' | string;
description?: string;
parent_span_id?: string;
data?: Record<string, string>;
op?: string;
};

export type Contexts = {
@@ -103,15 +105,15 @@ export type SentryErrorEvent = CommonEventAttrs & {
};

export type Span = {
trace_id: string;
trace_id?: string;
span_id: string;
parent_span_id?: string | null;
op?: string | null;
description?: string | null;
start_timestamp: number;
tags?: Tags | null;
timestamp: number;
status: 'ok' | string;
status?: 'ok' | string;
transaction?: SentryTransactionEvent;
children?: Span[];
data?: Record<string, unknown>;
@@ -192,11 +194,12 @@ export type SentryProfileV1Event = CommonEventAttrs & {
export type SentryEvent = SentryErrorEvent | SentryTransactionEvent | SentryProfileV1Event;

export type Trace = TraceContext & {
trace_id: string;
transactions: SentryTransactionEvent[];
errors: number;
start_timestamp: number;
timestamp: number;
status: string;
status?: 'ok' | string;
rootTransaction: SentryTransactionEvent | null;
rootTransactionName: string;
spans: Map<string, Span>;
Loading