Skip to content

Commit 0a83eae

Browse files
kamilogorekkamilogorek
committed
Next iteration of span API
1 parent 422efcd commit 0a83eae

File tree

5 files changed

+113
-111
lines changed

5 files changed

+113
-111
lines changed

‎packages/hub/src/hub.ts

+4-61
Original file line numberDiff line numberDiff line change
@@ -397,75 +397,18 @@ export class Hub implements HubInterface {
397397
/**
398398
* @inheritDoc
399399
*/
400-
public startSpan(spanContext?: SpanContext, bindOnScope: boolean = false): Span {
400+
public startSpan(spanContext?: SpanContext): Span {
401401
const top = this.getStackTop();
402402

403-
const simpleNewSpan = new Span(spanContext);
404-
if (top.scope && top.client) {
405-
if (bindOnScope) {
406-
top.scope.setSpan(simpleNewSpan);
407-
return simpleNewSpan;
408-
}
403+
if (top.scope) {
409404
const span = top.scope.getSpan();
405+
410406
if (span) {
411407
return span.newSpan(spanContext);
412408
}
413409
}
414410

415-
return simpleNewSpan;
416-
}
417-
418-
/**
419-
* @inheritDoc
420-
*/
421-
public finishSpan(span?: Span): string | undefined {
422-
const top = this.getStackTop();
423-
let passedSpan = span;
424-
425-
// If the passed span is undefined we try to get the span from the scope and finish it.
426-
if (passedSpan === undefined) {
427-
if (top.scope && top.client) {
428-
const scopeSpan = top.scope.getSpan();
429-
if (scopeSpan) {
430-
passedSpan = scopeSpan;
431-
}
432-
}
433-
}
434-
435-
if (passedSpan === undefined) {
436-
logger.warn('There was no Span on the Scope and none was passed, do nothing.');
437-
// We will do nothing since nothing was passed and there is no Span on the scope.
438-
return undefined;
439-
}
440-
441-
if (!passedSpan.timestamp) {
442-
passedSpan.finish();
443-
}
444-
445-
if (!passedSpan.transaction) {
446-
return undefined;
447-
}
448-
449-
if (!top.client) {
450-
return undefined;
451-
}
452-
453-
// TODO: if sampled do what?
454-
455-
const finishedSpans = passedSpan.finishedSpans.filter(s => s !== passedSpan);
456-
457-
const eventId = this.captureEvent({
458-
contexts: { trace: passedSpan.getTraceContext() },
459-
spans: finishedSpans.length > 0 ? finishedSpans : undefined,
460-
start_timestamp: passedSpan.startTimestamp,
461-
timestamp: passedSpan.timestamp,
462-
transaction: passedSpan.transaction,
463-
type: 'transaction',
464-
});
465-
466-
// After sending we reset the finishedSpans array
467-
passedSpan.finishedSpans = [];
468-
return eventId;
411+
return new Span(spanContext);
469412
}
470413
}
471414

‎packages/hub/src/span.ts

+35-2
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
import { Span as SpanInterface, SpanContext } from '@sentry/types';
22
import { timestampWithMs, uuid4 } from '@sentry/utils';
3+
import { getCurrentHub, Hub } from './hub';
34

45
export const TRACEPARENT_REGEXP = /^[ \t]*([0-9a-f]{32})?-?([0-9a-f]{16})?-?([01])?[ \t]*$/;
56

67
/**
78
* Span contains all data about a span
89
*/
910
export class Span implements SpanInterface, SpanContext {
11+
/**
12+
* @inheritDoc
13+
*/
14+
private readonly _hub: Hub = getCurrentHub();
15+
1016
/**
1117
* @inheritDoc
1218
*/
@@ -67,7 +73,11 @@ export class Span implements SpanInterface, SpanContext {
6773
*/
6874
public finishedSpans: Span[] = [];
6975

70-
public constructor(spanContext?: SpanContext) {
76+
public constructor(spanContext?: SpanContext, hub?: Hub) {
77+
if (hub instanceof Hub) {
78+
this._hub = hub;
79+
}
80+
7181
if (!spanContext) {
7282
return this;
7383
}
@@ -147,9 +157,32 @@ export class Span implements SpanInterface, SpanContext {
147157
/**
148158
* Sets the finish timestamp on the current span
149159
*/
150-
public finish(): void {
160+
public finish(): string | undefined {
161+
// Don't allow for finishing more than once
162+
if (typeof this.timestamp === 'number') {
163+
return undefined;
164+
}
165+
151166
this.timestamp = timestampWithMs();
152167
this.finishedSpans.push(this);
168+
169+
// Don't send non-transaction spans
170+
if (typeof this.transaction !== 'string') {
171+
return undefined;
172+
}
173+
174+
// TODO: if sampled do what?
175+
const finishedSpans = this.finishedSpans.filter(s => s !== this);
176+
this.finishedSpans = [];
177+
178+
return this._hub.captureEvent({
179+
contexts: { trace: this.getTraceContext() },
180+
spans: finishedSpans.length > 0 ? finishedSpans : undefined,
181+
start_timestamp: this.startTimestamp,
182+
timestamp: this.timestamp,
183+
transaction: this.transaction,
184+
type: 'transaction',
185+
});
153186
}
154187

155188
/**

‎packages/hub/test/hub.test.ts

+8-43
Original file line numberDiff line numberDiff line change
@@ -290,51 +290,16 @@ describe('Hub', () => {
290290
expect(span._spanId).toBeTruthy();
291291
});
292292

293-
test('bindOnScope', () => {
293+
test('inherits from parent span', () => {
294294
const myScope = new Scope();
295295
const hub = new Hub(clientFn, myScope);
296-
const span = hub.startSpan({}, true) as any;
297-
expect((myScope as any)._span).toBe(span);
298-
});
299-
});
300-
301-
describe('finish', () => {
302-
test('simple', () => {
303-
const hub = new Hub(clientFn);
304-
const span = hub.startSpan() as any;
305-
expect(span.timestamp).toBeUndefined();
306-
expect(hub.finishSpan(span)).toBeUndefined();
307-
expect(span.timestamp).toBeGreaterThan(1);
308-
});
309-
310-
test('finish a scope span without transaction', () => {
311-
const myScope = new Scope();
312-
const hub = new Hub(clientFn, myScope);
313-
const spy = jest.spyOn(hub as any, 'captureEvent');
314-
const span = hub.startSpan({}, true) as any;
315-
expect(hub.finishSpan(span)).toBeUndefined();
316-
expect(spy).not.toHaveBeenCalled();
317-
});
318-
319-
test('finish a scope span with transaction', () => {
320-
const myScope = new Scope();
321-
const hub = new Hub(clientFn, myScope);
322-
const spy = jest.spyOn(hub as any, 'captureEvent');
323-
const span = hub.startSpan({ transaction: 'test' }, true) as any;
324-
expect(hub.finishSpan(span)).toBeDefined();
325-
expect(spy).toHaveBeenCalled();
326-
expect(spy.mock.calls[0][0].spans).toBeUndefined();
327-
});
328-
329-
test('finish a scope span with transaction + child span', () => {
330-
const myScope = new Scope();
331-
const hub = new Hub(clientFn, myScope);
332-
const spy = jest.spyOn(hub as any, 'captureEvent');
333-
const span = hub.startSpan({ transaction: 'test' }, true) as any;
334-
hub.finishSpan(hub.startSpan());
335-
expect(hub.finishSpan(span)).toBeDefined();
336-
expect(spy).toHaveBeenCalled();
337-
expect(spy.mock.calls[0][0].spans).toHaveLength(1);
296+
const parentSpan = hub.startSpan({}) as any;
297+
expect(parentSpan._parentId).toBeFalsy();
298+
hub.configureScope(scope => {
299+
scope.setSpan(parentSpan);
300+
});
301+
const span = hub.startSpan({}) as any;
302+
expect(span._parentSpanId).toBeTruthy();
338303
});
339304
});
340305
});

‎packages/hub/test/scope.test.ts

+16-4
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Event, EventHint, Severity } from '@sentry/types';
22
import { getGlobalObject } from '@sentry/utils';
33

4-
import { addGlobalEventProcessor, Scope } from '../src';
4+
import { addGlobalEventProcessor, Scope, Span } from '../src';
55

66
describe('Scope', () => {
77
afterEach(() => {
@@ -73,9 +73,6 @@ describe('Scope', () => {
7373
scope.addBreadcrumb({ message: 'test' }, 100);
7474
expect((scope as any)._breadcrumbs[0]).toHaveProperty('message', 'test');
7575
});
76-
});
77-
78-
describe('level', () => {
7976
test('set', () => {
8077
const scope = new Scope();
8178
scope.setLevel(Severity.Critical);
@@ -111,6 +108,21 @@ describe('Scope', () => {
111108
});
112109
});
113110

111+
describe('span', () => {
112+
test('set', () => {
113+
const scope = new Scope();
114+
const span = new Span({});
115+
scope.setSpan(span);
116+
expect((scope as any)._span).toEqual(span);
117+
});
118+
test('unset', () => {
119+
const scope = new Scope();
120+
scope.setSpan(new Span({}));
121+
scope.setSpan();
122+
expect((scope as any)._span).toEqual(undefined);
123+
});
124+
});
125+
114126
test('chaining', () => {
115127
const scope = new Scope();
116128
scope.setLevel(Severity.Critical).setUser({ id: '1' });

‎packages/hub/test/span.test.ts

+50-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1-
import { Span, TRACEPARENT_REGEXP } from '../src';
1+
import { Span, TRACEPARENT_REGEXP, Hub, Scope } from '../src';
22

33
describe('Span', () => {
4+
let hub: Hub;
5+
6+
beforeEach(() => {
7+
const clientFn: any = jest.fn();
8+
const myScope = new Scope();
9+
hub = new Hub(clientFn, myScope);
10+
});
11+
412
describe('newSpan', () => {
513
test('simple', () => {
614
const span = new Span({ sampled: true });
@@ -9,6 +17,13 @@ describe('Span', () => {
917
expect((span2 as any)._traceId).toBe((span as any)._traceId);
1018
expect((span2 as any).sampled).toBe((span as any).sampled);
1119
});
20+
21+
test.only('gets currentHub', () => {
22+
const span = new Span({});
23+
const span2 = span.newSpan();
24+
expect((span as any)._hub).toBeInstanceOf(Hub);
25+
expect((span2 as any)._hub).toBeInstanceOf(Hub);
26+
});
1227
});
1328

1429
describe('toTraceparent', () => {
@@ -74,4 +89,38 @@ describe('Span', () => {
7489
expect(serialized).toHaveProperty('trace_id', 'c');
7590
});
7691
});
92+
93+
describe('finish', () => {
94+
test('simple', () => {
95+
const span = new Span({});
96+
expect(span.timestamp).toBeUndefined();
97+
span.finish();
98+
expect(span.timestamp).toBeGreaterThan(1);
99+
});
100+
101+
test('finish a scope span without transaction', () => {
102+
const spy = jest.spyOn(hub as any, 'captureEvent');
103+
const span = new Span({}, hub);
104+
span.finish();
105+
expect(spy).not.toHaveBeenCalled();
106+
});
107+
108+
test('finish a scope span with transaction', () => {
109+
const spy = jest.spyOn(hub as any, 'captureEvent') as any;
110+
const span = new Span({ transaction: 'test' }, hub);
111+
span.finish();
112+
expect(spy).toHaveBeenCalled();
113+
expect(spy.mock.calls[0][0].spans).toBeUndefined();
114+
});
115+
116+
test('finish a scope span with transaction + child span', () => {
117+
const spy = jest.spyOn(hub as any, 'captureEvent') as any;
118+
const parentSpan = new Span({ transaction: 'test' }, hub);
119+
const childSpan = parentSpan.newSpan();
120+
childSpan.finish();
121+
parentSpan.finish();
122+
expect(spy).toHaveBeenCalled();
123+
expect(spy.mock.calls[0][0].spans).toHaveLength(1);
124+
});
125+
});
77126
});

0 commit comments

Comments
 (0)