Skip to content

Commit dd0180a

Browse files
committed
apply review feedback
1 parent 021e6f6 commit dd0180a

5 files changed

Lines changed: 28 additions & 14 deletions

File tree

packages/browser/src/integrations/spanstreaming.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export const spanStreamingIntegration = defineIntegration(() => {
2424
},
2525

2626
setup(client) {
27-
const initialMessage = 'spanStreamingIntegration requires';
27+
const initialMessage = 'SpanStreaming integration requires';
2828
const fallbackMsg = 'Falling back to static trace lifecycle.';
2929

3030
if (!hasSpanStreamingEnabled(client)) {
@@ -37,7 +37,8 @@ export const spanStreamingIntegration = defineIntegration(() => {
3737
// using an incompatible beforeSendSpan callback, we fall back to the static trace lifecycle.
3838
if (beforeSendSpan && !isStreamedBeforeSendSpanCallback(beforeSendSpan)) {
3939
client.getOptions().traceLifecycle = 'static';
40-
debug.warn(`${initialMessage} a beforeSendSpan callback using \`withStreamSpan\`! ${fallbackMsg}`);
40+
DEBUG_BUILD &&
41+
debug.warn(`${initialMessage} a beforeSendSpan callback using \`withStreamedSpan\`! ${fallbackMsg}`);
4142
return;
4243
}
4344

packages/browser/test/integrations/spanstreaming.test.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ describe('spanStreamingIntegration', () => {
5959
client.init();
6060

6161
expect(debugSpy).toHaveBeenCalledWith(
62-
'spanStreamingIntegration requires `traceLifecycle` to be set to "stream"! Falling back to static trace lifecycle.',
62+
'SpanStreaming integration requires `traceLifecycle` to be set to "stream"! Falling back to static trace lifecycle.',
6363
);
6464
debugSpy.mockRestore();
6565

@@ -80,14 +80,14 @@ describe('spanStreamingIntegration', () => {
8080
client.init();
8181

8282
expect(debugSpy).toHaveBeenCalledWith(
83-
'spanStreamingIntegration requires a beforeSendSpan callback using `withStreamSpan`! Falling back to static trace lifecycle.',
83+
'SpanStreaming integration requires a beforeSendSpan callback using `withStreamedSpan`! Falling back to static trace lifecycle.',
8484
);
8585
debugSpy.mockRestore();
8686

8787
expect(client.getOptions().traceLifecycle).toBe('static');
8888
});
8989

90-
it('enqueues a span into the buffer when the span ends', () => {
90+
it('does nothing if traceLifecycle set to "stream"', () => {
9191
const client = new BrowserClient({
9292
...getDefaultBrowserClientOptions(),
9393
dsn: 'https://username@domain/123',
@@ -98,6 +98,19 @@ describe('spanStreamingIntegration', () => {
9898
SentryCore.setCurrentClient(client);
9999
client.init();
100100

101+
expect(client.getOptions().traceLifecycle).toBe('stream');
102+
});
103+
104+
it('enqueues a span into the buffer when the span ends', () => {
105+
const client = new BrowserClient({
106+
...getDefaultBrowserClientOptions(),
107+
dsn: 'https://username@domain/123',
108+
integrations: [spanStreamingIntegration()],
109+
});
110+
111+
SentryCore.setCurrentClient(client);
112+
client.init();
113+
101114
const span = new SentryCore.SentrySpan({ name: 'test' });
102115
client.emit('afterSpanEnd', span);
103116

packages/core/src/client.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -617,13 +617,13 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
617617
* Register a callback for after a span is ended and the `spanEnd` hook has run.
618618
* NOTE: The span cannot be mutated anymore in this callback.
619619
*/
620-
public on(hook: 'afterSpanEnd', callback: (span: Span) => void): () => void;
620+
public on(hook: 'afterSpanEnd', callback: (immutableSegmentSpan: Readonly<Span>) => void): () => void;
621621

622622
/**
623623
* Register a callback for after a segment span is ended and the `segmentSpanEnd` hook has run.
624624
* NOTE: The segment span cannot be mutated anymore in this callback.
625625
*/
626-
public on(hook: 'afterSegmentSpanEnd', callback: (segmentSpan: Span) => void): () => void;
626+
public on(hook: 'afterSegmentSpanEnd', callback: (immutableSegmentSpan: Readonly<Span>) => void): () => void;
627627

628628
/**
629629
* Register a callback for when a span JSON is processed, to add some data to the span JSON.
@@ -908,14 +908,14 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
908908
public emit(hook: 'spanEnd', span: Span): void;
909909

910910
/**
911-
* Fire a hook event when a span ends.
911+
* Fire a hook event after a span ends and the `spanEnd` hook has run.
912912
*/
913-
public emit(hook: 'afterSpanEnd', span: Span): void;
913+
public emit(hook: 'afterSpanEnd', immutableSpan: Readonly<Span>): void;
914914

915915
/**
916-
* Fire a hook event when a segment span ends.
916+
* Fire a hook event after a segment span ends and the `spanEnd` hook has run.
917917
*/
918-
public emit(hook: 'afterSegmentSpanEnd', segmentSpan: Span): void;
918+
public emit(hook: 'afterSegmentSpanEnd', immutableSegmentSpan: Readonly<Span>): void;
919919

920920
/**
921921
* Fire a hook event when a span JSON is processed, to add some data to the span JSON.

packages/core/src/tracing/spans/captureSpan.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export function captureSpan(span: Span, client: Client): SerializedStreamedSpanW
5151

5252
applyCommonSpanAttributes(spanJSON, serializedSegmentSpan, client, finalScopeData);
5353

54-
if (span === segmentSpan) {
54+
if (spanJSON.is_segment) {
5555
applyScopeToSegmentSpan(spanJSON, finalScopeData);
5656
// Allow hook subscribers to mutate the segment span JSON
5757
client.emit('processSegmentSpan', spanJSON);

packages/core/test/lib/utils/beforeSendSpan.test.ts renamed to packages/core/test/lib/tracing/spans/beforeSendSpan.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { describe, expect, it, vi } from 'vitest';
2-
import { withStreamedSpan } from '../../../src';
3-
import { isStreamedBeforeSendSpanCallback } from '../../../src/tracing/spans/beforeSendSpan';
2+
import { withStreamedSpan } from '../../../../src';
3+
import { isStreamedBeforeSendSpanCallback } from '../../../../src/tracing/spans/beforeSendSpan';
44

55
describe('beforeSendSpan for span streaming', () => {
66
describe('withStreamedSpan', () => {

0 commit comments

Comments
 (0)