Skip to content

Commit af0f6d7

Browse files
committed
apply review feedback
1 parent d519344 commit af0f6d7

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
@@ -621,13 +621,13 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
621621
* Register a callback for after a span is ended and the `spanEnd` hook has run.
622622
* NOTE: The span cannot be mutated anymore in this callback.
623623
*/
624-
public on(hook: 'afterSpanEnd', callback: (span: Span) => void): () => void;
624+
public on(hook: 'afterSpanEnd', callback: (immutableSegmentSpan: Readonly<Span>) => void): () => void;
625625

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

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

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

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

924924
/**
925925
* 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)