Skip to content

Commit

Permalink
fix: Handle missing root transactions
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 5c09149 commit e56ea36
Show file tree
Hide file tree
Showing 9 changed files with 681 additions and 19 deletions.
24 changes: 24 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
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
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
42 changes: 42 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,42 @@
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(),
parent_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('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,
});
const result = groupSpans([span1, span2]);
expect(result.length).toEqual(1);
expect(result[0].op).toEqual('unknown');
expect(result[0].status).toEqual('unknown');
expect(result[0].parent_span_id).toBe(null);
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);
});
});
34 changes: 31 additions & 3 deletions packages/core/src/integrations/sentry/utils/traces.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,24 @@
import { Span } from '../types';

function createFauxParent(
span: Span & {
parent_span_id: string;
},
): Span & {
parent_span_id: null;
} {
return {
trace_id: span.trace_id,
span_id: span.parent_span_id,
parent_span_id: null,
op: 'unknown',
// TODO: timestamps should represent whats encapsulated by children
start_timestamp: new Date().getTime(),
timestamp: new Date().getTime(),
status: 'unknown',
};
}

// mutates spans in place and adds children, as well as returns the top level tree
export function groupSpans(spans: Span[]) {
// ordered
Expand All @@ -17,10 +36,19 @@ export function groupSpans(spans: Span[]) {
if (!parent.children) parent.children = [];
parent.children.push(span);
} else if (span.parent_span_id) {
const parentParent = sortedSpans.find(s => !s.parent_span_id);
let parentParent = sortedSpans.find(s => !s.parent_span_id);
if (!parentParent) {
console.log('Cant create a faux parent');
return;
console.log(
`Unable to identify a faux parent (${span.parent_span_id}) for span (${span.span_id}) - root span not found. Creating imitation.`,
);
// XXX(dcramer): no idea why TSC thinks parent_span_id is not defined
parentParent = createFauxParent(
span as Span & {
parent_span_id: string;
},
);
tree.push(parentParent);
sortedSpans.unshift(parentParent);
}
parent = {
trace_id: span.trace_id,
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 e56ea36

Please sign in to comment.