Skip to content

Commit

Permalink
fix: Handle missing root transactions (#93)
Browse files Browse the repository at this point in the history
There are (currently unknown) situations where a span batch may not be able to identify a root transaction. That is, all spans available in the transaction group are siblings.

This may not be the ideal fix, and we'll revisit shortly. This is simply to unblock UI errors.

Refs GH-90, GH-61
  • Loading branch information
dcramer committed Nov 21, 2023
1 parent e96a5c6 commit 055fc9b
Show file tree
Hide file tree
Showing 9 changed files with 760 additions and 22 deletions.
33 changes: 33 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
name: Test

on:
push:
branches: [main]
pull_request:

jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Install pnpm
uses: pnpm/action-setup@v2
- name: Setup Node
uses: actions/setup-node@v3
- name: Install deps
run: pnpm install
- name: Build packages
run: pnpm build
- name: Run tests
run: pnpm test:ci
- name: Publish Test Report
uses: mikepenz/action-junit-report@v4
if: success() || failure()
with:
report_paths: '**/junit.xml'
- name: Upload coverage reports to Codecov
uses: codecov/codecov-action@v3
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,6 @@ dist-ssr
.yalc
yalc.lock
.vite-inspect
.vercel
.vercel

junit.xml
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
"preinstall": "npx only-allow pnpm",
"postinstall": "simple-git-hooks",
"format": "prettier --write --cache .",
"test": "pnpm -r --stream --parallel run test",
"test:ci": "CI=true pnpm -r run test:ci",
"yalc:publish": "yalc publish --push --sig --private",
"changeset:add": "pnpm changeset",
"changeset:consume": "pnpm changeset version",
Expand Down
9 changes: 7 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
"build": "tsc && vite build",
"build:watch": "vite build --watch",
"preview": "vite preview",
"test": "vitest",
"test:ci": "vitest --coverage --reporter=junit --reporter=default --outputFile=junit.xml",
"yalc:publish": "yalc publish --push --sig --private"
},
"files": [
Expand Down Expand Up @@ -36,21 +38,24 @@
"usehooks-ts": "^2.9.1"
},
"devDependencies": {
"@spotlightjs/tsconfig": "workspace:*",
"@paralleldrive/cuid2": "^2.2.2",
"@sentry/types": "^7.77.0",
"@spotlightjs/tsconfig": "workspace:*",
"@tailwindcss/forms": "^0.5.4",
"@types/react": "^18.2.37",
"@types/react-dom": "^18.2.15",
"@typescript-eslint/eslint-plugin": "^6.0.0",
"@typescript-eslint/parser": "^6.0.0",
"@vitejs/plugin-react": "^4.0.3",
"@vitest/coverage-v8": "^0.34.6",
"eslint": "^8.45.0",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-react-refresh": "^0.4.3",
"typescript": "^5.0.2",
"vite": "^4.4.5",
"vite-plugin-dts": "^3.5.2",
"vite-plugin-svgr": "^3.2.0"
"vite-plugin-svgr": "^3.2.0",
"vitest": "^0.34.6"
},
"volta": {
"extends": "../../package.json"
Expand Down
130 changes: 130 additions & 0 deletions packages/core/src/integrations/sentry/utils/traces.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import { createId } from '@paralleldrive/cuid2';
import { describe, expect, test } from 'vitest';
import { Span } from '../types';
import { groupSpans } from './traces';

function mockSpan({ duration, ...span }: Partial<Span> & { duration?: number } = {}): Span {
const defaultTimestamp = new Date().getTime();
return {
trace_id: createId(),
span_id: createId(),
op: 'unknown',
status: 'unknown',
start_timestamp: defaultTimestamp,
timestamp: duration ? (span.start_timestamp || defaultTimestamp) + duration : defaultTimestamp,
...span,
};
}

describe('groupSpans', () => {
test('empty span list', () => {
expect(groupSpans([])).toEqual([]);
});

test('simple parent and child relationship', () => {
const parent1 = mockSpan({});
const span1 = mockSpan({
parent_span_id: parent1.span_id,
trace_id: parent1.trace_id,
});
const span2 = mockSpan({
parent_span_id: parent1.span_id,
trace_id: parent1.trace_id,
});
const result = groupSpans([parent1, span1, span2]);
console.debug(result);
expect(result.length).toEqual(1);
expect(result[0].span_id).toEqual(parent1.span_id);
expect(result[0].children?.length).toEqual(2);
expect(result[0].children![0].span_id).toEqual(span1.span_id);
expect(result[0].children![1].span_id).toEqual(span2.span_id);
});

test('multiple parent and child relationship both parents as root', () => {
const parent1 = mockSpan({
start_timestamp: new Date().getTime() - 1,
});

const span1 = mockSpan({
parent_span_id: parent1.span_id,
trace_id: parent1.trace_id,
});
const span2 = mockSpan({
parent_span_id: parent1.span_id,
trace_id: parent1.trace_id,
});

const parent2 = mockSpan({});
const span3 = mockSpan({
parent_span_id: parent2.span_id,
trace_id: parent2.trace_id,
});
const span4 = mockSpan({
parent_span_id: parent2.span_id,
trace_id: parent2.trace_id,
});
const result = groupSpans([parent1, span1, span2, parent2, span3, span4]);
console.debug(result);
expect(result.length).toEqual(2);
expect(result[0].span_id).toEqual(parent1.span_id);
expect(result[0].children?.length).toEqual(2);
expect(result[0].children![0].span_id).toEqual(span1.span_id);
expect(result[0].children![1].span_id).toEqual(span2.span_id);
expect(result[1].span_id).toEqual(parent2.span_id);
expect(result[1].children?.length).toEqual(2);
expect(result[1].children![0].span_id).toEqual(span3.span_id);
expect(result[1].children![1].span_id).toEqual(span4.span_id);
});

test('missing root transactions as siblings, creates faux parent', () => {
const parent_span_id = createId();
const span1 = mockSpan({
parent_span_id,
});
const span2 = mockSpan({
parent_span_id,
trace_id: span1.trace_id,
});
const result = groupSpans([span1, span2]);
console.debug(result);
expect(result.length).toEqual(1);
expect(result[0].op).toEqual('orphan');
expect(result[0].description).toEqual('missing or unknown parent span');
expect(result[0].status).toEqual('unknown');
expect(result[0].parent_span_id).toBe(null);
console.debug(result[0].children);
expect(result[0].children?.length).toEqual(2);
expect(result[0].children![0].span_id).toEqual(span1.span_id);
expect(result[0].children![1].span_id).toEqual(span2.span_id);
});

test('missing root transactions as independent children, creates faux parents', () => {
const span1 = mockSpan({
parent_span_id: createId(),
});
const span2 = mockSpan({
parent_span_id: createId(),
trace_id: span1.trace_id,
});
const result = groupSpans([span1, span2]);
console.debug(result);
expect(result.length).toEqual(2);
expect(result[0].span_id).toEqual(span1.parent_span_id);
expect(result[0].op).toEqual('orphan');
expect(result[0].description).toEqual('missing or unknown parent span');
expect(result[0].status).toEqual('unknown');
expect(result[0].parent_span_id).toBe(null);
console.debug(result[0].children);
expect(result[0].children?.length).toEqual(1);
expect(result[0].children![0].span_id).toEqual(span1.span_id);

expect(result[1].span_id).toEqual(span2.parent_span_id);
expect(result[1].op).toEqual('orphan');
expect(result[1].description).toEqual('missing or unknown parent span');
expect(result[1].status).toEqual('unknown');
expect(result[1].parent_span_id).toBe(null);
console.debug(result[1].children);
expect(result[1].children?.length).toEqual(1);
expect(result[1].children![0].span_id).toEqual(span2.span_id);
});
});
19 changes: 13 additions & 6 deletions packages/core/src/integrations/sentry/utils/traces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,29 @@ export function groupSpans(spans: Span[]) {
} else if (span.parent_span_id) {
const parentParent = sortedSpans.find(s => !s.parent_span_id);
if (!parentParent) {
console.log('Cant create a faux parent');
return;
console.log(`Root span (${span.parent_span_id}) for span (${span.span_id}). Creating orphan.`);
} else {
console.log(`Creating orphan for parent (${span.parent_span_id}) for span (${span.span_id})`);
}
parent = {
trace_id: span.trace_id,
span_id: span.parent_span_id,
parent_span_id: parentParent.span_id,
parent_span_id: parentParent ? parentParent.span_id : null,
op: 'orphan',
description: 'missing parent span',
description: 'missing or unknown parent span',
children: [span],
start_timestamp: span.start_timestamp,
timestamp: span.timestamp,
status: 'unknown',
};
if (!parentParent.children) parentParent.children = [];
parentParent.children.push(parent);
idLookup.set(parent.span_id, parent);
// sortedSpans.splice(spanIdx, 0, parent);
if (parentParent) {
if (!parentParent.children) parentParent.children = [];
parentParent.children.push(parent);
} else {
tree.push(parent);
}
} else {
tree.push(span);
}
Expand Down
18 changes: 18 additions & 0 deletions packages/core/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference types="vitest" />

// import tsconfigPaths from "vite-tsconfig-paths";
import { defineConfig } from 'vitest/config';

export default defineConfig({
// plugins: [tsconfigPaths()],
test: {
coverage: {
provider: 'v8',
reporter: ['json'],
},
globals: true,
// setupFiles: ["./src/test/setup-test-env.ts"],
include: ['./src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'],
watchExclude: ['.*\\/node_modules\\/.*', '.*\\/dist\\/.*'],
},
});
Empty file modified packages/sidecar/server.js
100644 → 100755
Empty file.
Loading

0 comments on commit 055fc9b

Please sign in to comment.