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

feat: Start using profile information in span tree #692

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
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
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()`
7 changes: 7 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,13 @@ jobs:

- name: Run tests
run: pnpm test:e2e

- name: Test results
if: always()
uses: actions/upload-artifact@v4
with:
name: test-results
path: e2e-tests/**/test-results/**
docker:
name: Docker Image
needs: build
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ dist-bin
node
dist-ssr
*.local
test-results

.eslintcache

Expand Down
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 @@
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 @@

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 @@
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 @@ -6,25 +6,25 @@
import classNames from '../../../../../lib/classNames';
import { useSpotlightContext } from '../../../../../lib/useSpotlightContext';
import { useSentryHelpers } from '../../../data/useSentryHelpers';
import { useSentryTraces } from '../../../data/useSentryTraces';
import { useSentryTraces } from '../../../data/useSentrySpans';
import { getFormattedSpanDuration } from '../../../utils/duration';
import { truncateId } from '../../../utils/text';
import HiddenItemsButton from '../../HiddenItemsButton';
import { TraceRootTxnName } from './TraceDetails/components/TraceRootTxnName';
import TraceIcon from './TraceIcon';

export default function TraceList() {
const traceList = useSentryTraces();
const { allTraces, localTraces } = useSentryTraces();

Check warning on line 17 in packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx#L17

Added line #L17 was not covered by tests
const helpers = useSentryHelpers();
const context = useSpotlightContext();

const [showAll, setShowAll] = useState(!context.experiments['sentry:focus-local-events']);
const filteredTraces = showAll ? traceList : traceList.filter(t => helpers.isLocalToSession(t.trace_id) !== false);
const hiddenItemCount = traceList.length - filteredTraces.length;
const filteredTraces = showAll ? allTraces : localTraces;
const hiddenItemCount = allTraces.length - filteredTraces.length;

Check warning on line 23 in packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx#L22-L23

Added lines #L22 - L23 were not covered by tests

return (
<>
{traceList.length !== 0 ? (
{allTraces.length !== 0 ? (

Check warning on line 27 in packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx

View check run for this annotation

Codecov / codecov/patch

packages/overlay/src/integrations/sentry/components/explore/traces/TraceList.tsx#L27

Added line #L27 was not covered by tests
<CardList>
{hiddenItemCount > 0 && (
<HiddenItemsButton
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
'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
Loading