-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(node-core): Add POtel server-side span streaming implementation #19741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: lms/feat-span-first
Are you sure you want to change the base?
Changes from all commits
52c06cb
0a989e3
e3e9a86
17df959
e8c9f04
3978348
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| import type { IntegrationFn } from '../types-hoist/integration'; | ||
| import { DEBUG_BUILD } from '../debug-build'; | ||
| import { defineIntegration } from '../integration'; | ||
| import { isStreamedBeforeSendSpanCallback } from '../tracing/spans/beforeSendSpan'; | ||
| import { captureSpan } from '../tracing/spans/captureSpan'; | ||
| import { hasSpanStreamingEnabled } from '../tracing/spans/hasSpanStreamingEnabled'; | ||
| import { SpanBuffer } from '../tracing/spans/spanBuffer'; | ||
| import { debug } from '../utils/debug-logger'; | ||
| import { spanIsSampled } from '../utils/spanUtils'; | ||
|
|
||
| export const spanStreamingIntegration = defineIntegration(() => { | ||
| return { | ||
| name: 'SpanStreaming', | ||
|
|
||
| setup(client) { | ||
| const initialMessage = 'SpanStreaming integration requires'; | ||
| const fallbackMsg = 'Falling back to static trace lifecycle.'; | ||
|
|
||
| if (!hasSpanStreamingEnabled(client)) { | ||
| DEBUG_BUILD && debug.warn(`${initialMessage} \`traceLifecycle\` to be set to "stream"! ${fallbackMsg}`); | ||
| return; | ||
| } | ||
|
|
||
| const beforeSendSpan = client.getOptions().beforeSendSpan; | ||
| if (beforeSendSpan && !isStreamedBeforeSendSpanCallback(beforeSendSpan)) { | ||
| client.getOptions().traceLifecycle = 'static'; | ||
| DEBUG_BUILD && | ||
| debug.warn(`${initialMessage} a beforeSendSpan callback using \`withStreamedSpan\`! ${fallbackMsg}`); | ||
| return; | ||
| } | ||
|
|
||
| const buffer = new SpanBuffer(client); | ||
|
|
||
| client.on('afterSpanEnd', span => { | ||
| if (!spanIsSampled(span)) { | ||
| return; | ||
| } | ||
| buffer.add(captureSpan(span, client)); | ||
| }); | ||
| }, | ||
| }; | ||
| }) satisfies IntegrationFn; | ||
Lms24 marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate spanStreamingIntegration logic between core and browserLow Severity The new Additional Locations (1)Triggered by project rule: PR Review Guidelines for Cursor Bot |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| import * as SentryCore from '../../src'; | ||
| import { debug } from '../../src'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { spanStreamingIntegration } from '../../src/integrations/spanStreaming'; | ||
| import { TestClient, getDefaultTestClientOptions } from '../mocks/client'; | ||
|
|
||
| const mockSpanBufferInstance = vi.hoisted(() => ({ | ||
| flush: vi.fn(), | ||
| add: vi.fn(), | ||
| drain: vi.fn(), | ||
| })); | ||
|
|
||
| const MockSpanBuffer = vi.hoisted(() => { | ||
| return vi.fn(() => mockSpanBufferInstance); | ||
| }); | ||
|
|
||
| vi.mock('../../src/tracing/spans/spanBuffer', async () => { | ||
| const original = await vi.importActual('../../src/tracing/spans/spanBuffer'); | ||
| return { | ||
| ...original, | ||
| SpanBuffer: MockSpanBuffer, | ||
| }; | ||
| }); | ||
|
|
||
| describe('spanStreamingIntegration (core)', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('has the correct name and setup hook', () => { | ||
| const integration = spanStreamingIntegration(); | ||
| expect(integration.name).toBe('SpanStreaming'); | ||
| // eslint-disable-next-line @typescript-eslint/unbound-method | ||
| expect(integration.setup).toBeDefined(); | ||
| }); | ||
|
|
||
| it('logs a warning if traceLifecycle is not set to "stream"', () => { | ||
| const debugSpy = vi.spyOn(debug, 'warn').mockImplementation(() => {}); | ||
| const client = new TestClient({ | ||
| ...getDefaultTestClientOptions(), | ||
| dsn: 'https://username@domain/123', | ||
| integrations: [spanStreamingIntegration()], | ||
| traceLifecycle: 'static', | ||
| }); | ||
|
|
||
| SentryCore.setCurrentClient(client); | ||
| client.init(); | ||
|
|
||
| expect(debugSpy).toHaveBeenCalledWith( | ||
| 'SpanStreaming integration requires `traceLifecycle` to be set to "stream"! Falling back to static trace lifecycle.', | ||
| ); | ||
| debugSpy.mockRestore(); | ||
|
|
||
| expect(client.getOptions().traceLifecycle).toBe('static'); | ||
| }); | ||
|
|
||
| it('falls back to static trace lifecycle if beforeSendSpan is not compatible with span streaming', () => { | ||
| const debugSpy = vi.spyOn(debug, 'warn').mockImplementation(() => {}); | ||
| const client = new TestClient({ | ||
| ...getDefaultTestClientOptions(), | ||
| dsn: 'https://username@domain/123', | ||
| integrations: [spanStreamingIntegration()], | ||
| traceLifecycle: 'stream', | ||
| beforeSendSpan: (span: SentryCore.SpanJSON) => span, | ||
| }); | ||
|
|
||
| SentryCore.setCurrentClient(client); | ||
| client.init(); | ||
|
|
||
| expect(debugSpy).toHaveBeenCalledWith( | ||
| 'SpanStreaming integration requires a beforeSendSpan callback using `withStreamedSpan`! Falling back to static trace lifecycle.', | ||
| ); | ||
| debugSpy.mockRestore(); | ||
|
|
||
| expect(client.getOptions().traceLifecycle).toBe('static'); | ||
| }); | ||
|
|
||
| it('sets up buffer when traceLifecycle is "stream"', () => { | ||
| const client = new TestClient({ | ||
| ...getDefaultTestClientOptions(), | ||
| dsn: 'https://username@domain/123', | ||
| integrations: [spanStreamingIntegration()], | ||
| traceLifecycle: 'stream', | ||
| }); | ||
|
|
||
| SentryCore.setCurrentClient(client); | ||
| client.init(); | ||
|
|
||
| expect(MockSpanBuffer).toHaveBeenCalledWith(client); | ||
| expect(client.getOptions().traceLifecycle).toBe('stream'); | ||
| }); | ||
|
|
||
| it('enqueues a span into the buffer when the span ends', () => { | ||
| const client = new TestClient({ | ||
| ...getDefaultTestClientOptions(), | ||
| dsn: 'https://username@domain/123', | ||
| integrations: [spanStreamingIntegration()], | ||
| traceLifecycle: 'stream', | ||
| tracesSampleRate: 1, | ||
| }); | ||
|
|
||
| SentryCore.setCurrentClient(client); | ||
| client.init(); | ||
|
|
||
| const span = new SentryCore.SentrySpan({ name: 'test', sampled: true }); | ||
| client.emit('afterSpanEnd', span); | ||
|
|
||
| expect(mockSpanBufferInstance.add).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| _segmentSpan: span, | ||
| trace_id: span.spanContext().traceId, | ||
| span_id: span.spanContext().spanId, | ||
| name: 'test', | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| it('does not enqueue a span into the buffer when the span is not sampled', () => { | ||
| const client = new TestClient({ | ||
| ...getDefaultTestClientOptions(), | ||
| dsn: 'https://username@domain/123', | ||
| integrations: [spanStreamingIntegration()], | ||
| traceLifecycle: 'stream', | ||
| tracesSampleRate: 1, | ||
| }); | ||
|
|
||
| SentryCore.setCurrentClient(client); | ||
| client.init(); | ||
|
|
||
| const span = new SentryCore.SentrySpan({ name: 'test', sampled: false }); | ||
| client.emit('afterSpanEnd', span); | ||
|
|
||
| expect(mockSpanBufferInstance.add).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import type { Integration, Options } from '@sentry/core'; | ||
| import { applySdkMetadata, hasSpansEnabled } from '@sentry/core'; | ||
| import { applySdkMetadata, hasSpansEnabled, spanStreamingIntegration } from '@sentry/core'; | ||
| import type { NodeClient } from '@sentry/node-core'; | ||
| import { | ||
| getDefaultIntegrations as getNodeCoreDefaultIntegrations, | ||
|
|
@@ -33,6 +33,7 @@ export function getDefaultIntegrations(options: Options): Integration[] { | |
| // This means that generally request isolation will work (because that is done by httpIntegration) | ||
| // But `transactionName` will not be set automatically | ||
| ...(hasSpansEnabled(options) ? getAutoPerformanceIntegrations() : []), | ||
| ...(options.traceLifecycle === 'stream' ? [spanStreamingIntegration()] : []), | ||
| ]; | ||
| } | ||
|
|
||
|
Comment on lines
33
to
39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Suggested FixModify Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.