Skip to content

Commit

Permalink
feat: Start using profile information in span tree
Browse files Browse the repository at this point in the history
  • Loading branch information
BYK committed Feb 4, 2025
1 parent 1ffe90f commit 7f1ef89
Show file tree
Hide file tree
Showing 16 changed files with 421 additions and 44 deletions.
17 changes: 17 additions & 0 deletions .changeset/few-points-rush.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@spotlightjs/tsconfig': major
'@spotlightjs/spotlight': minor
'@spotlightjs/electron': minor
'@spotlightjs/overlay': minor
'@spotlightjs/astro': minor
---

# Add profile grafting into traces

With this change, Spotlight can now ingest v1 profiles and graft profiling
data into the trace view to fill in the gaps where span/trace instrumentation
falls short.

This feature is experimental.

Breaking change for `tsconfig`: It now targets ES2023 as we needed `Array.findLastIndex()`
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"Astro",
"Astro's",
"astrojs",
"backto",
"buildx",
"codesign",
"contextlines",
Expand Down
5 changes: 5 additions & 0 deletions demos/astro-playground/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ export default defineConfig({
adapter: node({
mode: 'standalone',
}),
server: {
headers: {
'Document-Policy': 'js-profiling',
},
},
integrations: [
svelte({ include: ['**/svelte/*'] }),
react({ include: ['**/react/*'] }),
Expand Down
2 changes: 2 additions & 0 deletions demos/astro-playground/sentry.client.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ import * as Sentry from '@sentry/astro';

Sentry.init({
debug: true,
integrations: [Sentry.browserProfilingIntegration()],
profilesSampleRate: 1.0,
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export default function EventList({ traceId }: { traceId?: string }) {
const helpers = useSentryHelpers();
const context = useSpotlightContext();

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

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

View check run for this annotation

Codecov / codecov/patch

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

Added line #L19 was not covered by tests

const [showAll, setShowAll] = useState(!context.experiments['sentry:focus-local-events']);
const filteredEvents = showAll
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default function TraceTreeview({ traceId }: TraceTreeViewProps) {

const [spanNodeWidth, setSpanNodeWidth] = useState<number>(DEFAULT_SPAN_NODE_WIDTH);

const trace = sentryDataCache.getTraceById(traceId);
const trace = sentryDataCache.getTraceById(traceId)!;

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/explore/traces/TraceDetails/components/TraceTreeview.tsx#L18

Added line #L18 was not covered by tests
const span = spanId ? sentryDataCache.getSpanById(traceId, spanId) : undefined;
const startTimestamp = trace.start_timestamp;
const totalDuration = trace.timestamp - startTimestamp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export default function TraceDetails() {
const errorCount = events.filter(
e =>
e.type !== 'transaction' &&
e.type !== 'profile' &&

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

View check run for this annotation

Codecov / codecov/patch

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const SpanItem = ({
'hover:bg-primary-900 group flex text-sm',
isQueried ? 'bg-primary-200 bg-opacity-20' : '',
spanId === span.span_id ? 'bg-primary-900' : '',
span.tags?.source === 'profile' ? 'text-lime-500' : '',

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/explore/traces/spans/SpanItem.tsx#L61

Added line #L61 was not covered by tests
)}
style={{
pointerEvents: isResizing ? 'none' : 'auto',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import classNames from '../../../../../../lib/classNames';
import { Span, TraceContext } from '../../../../types';
import type { Span, TraceContext } from '../../../../types';
import SpanItem from './SpanItem';

export default function SpanTree({
Expand All @@ -24,7 +24,6 @@ export default function SpanTree({
query?: string;
}) {
if (!tree || !tree.length) return null;

return (
<ul className={classNames(tree.length > 1 && 'deep', 'tree')}>
{tree.map(span => {
Expand Down
210 changes: 210 additions & 0 deletions packages/overlay/src/integrations/sentry/data/profiles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
import { log } from '~/lib/logger';
import { generateUuidv4 } from '../../../lib/uuid';
import type { Span, Trace } from '../types';
import { compareSpans } from '../utils/traces';
import type { SentryProfileWithTraceMeta } from './sentryDataCache';
import sentryDataCache from './sentryDataCache';

/**
* Groups consequent spans with the same description and op into a single span per each level.
* Essentially a BFS traversal of the spans tree.
* @param spans Span[] A list of spans to consolidate, sorted by their start_timestamp
* @returns Span[] A list of spans with the same description and op consolidated into a single span
*/
function consolidateSpans(trace: Trace, spans: Span[]): Span[] {
const consolidatedSpans: Span[] = [];
let lastSpan = spans[0];
let spanIdx = 1;
while (spanIdx < spans.length + 1) {
const span = spans[spanIdx];
spanIdx += 1;
if (span && span.description === lastSpan.description && span.op === lastSpan.op) {
// Require the spans are sorted by start_timestamp
lastSpan.timestamp = span.timestamp;
if (span.children) {
if (lastSpan.children) {
for (const child of span.children) {
lastSpan.children.push(child);
}
lastSpan.children.sort(compareSpans);
} else {
lastSpan.children = span.children;
}
}
} else {
// Discard very short spans
if (lastSpan.timestamp - lastSpan.start_timestamp > 0) {
if (lastSpan.children && lastSpan.children.length > 1) {
lastSpan.children = consolidateSpans(trace, lastSpan.children);
}
consolidatedSpans.push(lastSpan);
trace.spans.set(lastSpan.span_id, lastSpan);
}
lastSpan = span;
}
}

return consolidatedSpans;
}

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/profiles.ts#L14-L48

Added lines #L14 - L48 were not covered by tests

// This is done per platform right now but we may want to make it use
// SDK or something more specific, especially for JS as `javascript` may
// mean browser, node, etc.
const SENTRY_FRAME_FILTER_PER_PLATFORM: Record<
string,
(this: SentryProfileWithTraceMeta['frames'], frameIdx: number) => boolean | undefined
> = {
python: function (frameIdx) {
return this[frameIdx].module?.startsWith('sentry_sdk.');
},

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/profiles.ts#L58-L59

Added lines #L58 - L59 were not covered by tests
javascript: function (frameIdx) {
const frame = this[frameIdx];
const module = frame.module;
if (module) {
return module.startsWith('@sentry') || module.startsWith('@opentelemetry.instrumentation');
}
return frame.abs_path?.includes('node_modules/@sentry');
},

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/profiles.ts#L61-L67

Added lines #L61 - L67 were not covered by tests
};

export function getSpansFromProfile(
trace: Trace,
profile: SentryProfileWithTraceMeta,
parent_span_id: string | undefined,
startTs: number,
endTs: number,
threadIds: Set<string>,
): Span[] {
threadIds.add(profile.active_thread_id);

const sentryFrameFilter = profile.platform && SENTRY_FRAME_FILTER_PER_PLATFORM[profile.platform];
// Try to fill in the gaps from profile data
const fillerSpans: Span[] = [];
for (let sampleIdx = 0; sampleIdx < profile.samples.length; sampleIdx++) {
const sample = profile.samples[sampleIdx];
if (sample.thread_id && !threadIds.has(sample.thread_id)) {
continue;
}
const sampleTs = sample.start_timestamp;
if (sampleTs < startTs || sampleTs > endTs) {
continue;
}
const nextSample = profile.samples[sampleIdx + 1];
const timestamp = nextSample ? nextSample.start_timestamp : endTs;

if (timestamp > endTs) {
continue;
}
const commonAttributes = {
start_timestamp: sampleTs,
timestamp,
trace_id: trace.trace_id,
status: 'ok',
tags: { source: 'profile' },
data: {
'thread.id': sample.thread_id,
'thread.name': profile.thread_metadata[sample.thread_id as keyof typeof profile.thread_metadata]?.name,
},
};
const sampleSpan: Span = {
span_id: generateUuidv4(),
parent_span_id,
...commonAttributes,
op: 'Thread',
description:
profile.thread_metadata[sample.thread_id as keyof typeof profile.thread_metadata]?.name ||
`Thread ${sample.thread_id}`,
data: {
thread_id: sample.thread_id,
},
};
let currentSpan = sampleSpan;
const currentStack = profile.stacks[sample.stack_id];
const lastSentryFrameIdx = sentryFrameFilter ? currentStack.findLastIndex(sentryFrameFilter, profile.frames) : 0;
for (let frameIdxIdx = lastSentryFrameIdx + 1; frameIdxIdx < currentStack.length; frameIdxIdx++) {
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 = {
span_id: generateUuidv4(),
parent_span_id: currentSpan.span_id,
...commonAttributes,
op: frame.module,
description: frame.function || `<anonymous>@${frame.lineno}:${frame.colno}`,
data: {
...frame,
},
};
currentSpan.children = [spanFromFrame];
currentSpan = spanFromFrame;
}
fillerSpans.push(sampleSpan);
}

if (!fillerSpans.length) {
return [];
}
const consolidated = consolidateSpans(trace, fillerSpans);
// Remove the extra layer of nesting if there is only one span which should be the "Thread" span
return (consolidated.length === 1 ? consolidated[0].children || [] : consolidated).filter(
span => span.timestamp - span.start_timestamp > 0 && span.timestamp <= endTs,
);
}

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/profiles.ts#L71-L152

Added lines #L71 - L152 were not covered by tests

/**
* Modifies the spanTree in place recursively by adding spans from the
* profile data where there are gaps in the trace data.
* @param spanTree Span[] The tree of spans to graft profile spans into
*/
export function graftProfileSpans(
trace: Trace,
spanTree: Span[] = trace.spanTree,
parent: Span | Trace = trace,
profile?: SentryProfileWithTraceMeta,
) {
if (trace.profileGrafted) {
log(`Trace already has profile grafted ${trace.trace_id}`);
return;
}

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/profiles.ts#L166-L168

Added lines #L166 - L168 were not covered by tests
if (!profile) {
profile = sentryDataCache.getProfileByTraceId(trace.trace_id);
if (!profile) {
log(`Profile not found for trace ${trace.trace_id}`);
return;
}
}

let idx = -1;
while (idx < spanTree.length) {
const span = spanTree[idx] as Span | undefined;
if (span?.tags?.source === 'profile') {
idx += 1;
continue;
}
const nextSpan = spanTree[idx + 1];
if (nextSpan?.tags?.source === 'profile') {
idx += 1;
continue;
}

const startTs = span ? span.timestamp : parent.start_timestamp;
const endTs = nextSpan ? nextSpan.start_timestamp : parent.timestamp;
const threadIds = new Set([span?.data?.threadId, nextSpan?.data?.threadId, parent?.data?.threadId]);
threadIds.delete(undefined);
if (endTs - startTs > 0) {
const fillers = getSpansFromProfile(trace, profile, parent.span_id, startTs, endTs, threadIds as Set<string>);
if (fillers.length) {
spanTree.splice(idx + 1, 0, ...fillers);
idx += fillers.length;
}
}
if (span) {
span.children ??= [];
graftProfileSpans(trace, span.children, span, profile);
}
idx += 1;
}
// Only mark as grafted at the top level to avoid early quitting during
// recursive calls above for child spans
trace.profileGrafted = trace.spanTree === spanTree;
}

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

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/data/profiles.ts#L176-L210

Added lines #L176 - L210 were not covered by tests
Loading

0 comments on commit 7f1ef89

Please sign in to comment.