feat(node-core): Add POtel server-side span streaming implementation#19741
Open
Lms24 wants to merge 6 commits intolms/feat-span-firstfrom
Open
feat(node-core): Add POtel server-side span streaming implementation#19741Lms24 wants to merge 6 commits intolms/feat-span-firstfrom
Lms24 wants to merge 6 commits intolms/feat-span-firstfrom
Conversation
Lms24
changed the title
Lms24
commented
Mar 10, 2026
| */ | ||
| public onStart(span: Span, parentContext: Context): void { | ||
| onSpanStart(span, parentContext); | ||
| // This is a reliable way to get the parent span - because this is exactly how the parent is identified in the OTEL SDK |
Member
Author
There was a problem hiding this comment.
I only inlined the code here from onSpanStart. Not sure why we had this abstraction previously but maybe it was a leftover when onSpanStart was called from multiple call sites.
| /** @inheritDoc */ | ||
| public onEnd(span: Span & ReadableSpan): void { | ||
| onSpanEnd(span); | ||
| logSpanEnd(span); |
Member
Author
There was a problem hiding this comment.
I also inlined onSpanEnd here since the function wasn't called from anywhere else
Contributor
size-limit report 📦
|
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
Lms24
force-pushed
the
lms/feat-span-first
branch
from
6efdac3 to
78cf628
Compare
Lms24
force-pushed
the
lms/feat-span-first-node-otel
branch
from
e85814b to
e3e9a86
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/node-core/src/sdk/client.ts
Outdated
| _INTERNAL_flushLogsBuffer, | ||
| applySdkMetadata, | ||
| debug, | ||
| hasSpanStreamingEnabled, |
There was a problem hiding this comment.
Unused import of hasSpanStreamingEnabled in client
Low Severity
hasSpanStreamingEnabled is imported into client.ts but never used anywhere in the file. This appears to be a leftover from development — the entire file was reviewed and the symbol only appears on the import line. This is dead code introduced by this PR.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
33
to
39
| // 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()] : []), | ||
| ]; | ||
| } | ||
|
|
There was a problem hiding this comment.
Bug: The getDefaultIntegrationsWithoutPerformance function doesn't accept or forward options, preventing serverless SDKs from enabling span streaming via traceLifecycle: 'stream'.
Severity: MEDIUM
Suggested Fix
Modify getDefaultIntegrationsWithoutPerformance to accept an options parameter and pass it to getNodeCoreDefaultIntegrations(). Update callers of getDefaultIntegrationsWithoutPerformance, such as in packages/aws-serverless/src/init.ts, to pass their received _options parameter.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/node/src/sdk/index.ts#L33-L39
Potential issue: The `getDefaultIntegrationsWithoutPerformance` function in
`@sentry/node` is called without any arguments. This function, in turn, calls
`getNodeCoreDefaultIntegrations()` without forwarding any options. As a result, when
serverless SDKs like `@sentry/aws-serverless` and `@sentry/google-cloud-serverless` use
this function, the `traceLifecycle: 'stream'` option set during `Sentry.init()` is
ignored. This prevents the `spanStreamingIntegration` from being added, effectively
disabling the span streaming feature for these environments despite it being a
documented public option.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Duplicate spanStreamingIntegration logic between core and browser
- Extracted common setup logic into setupSpanStreaming helper function in core, eliminating ~25 lines of duplicated code while preserving browser-specific features.
Or push these changes by commenting:
@cursor push 7d214a5dfb
Preview (7d214a5dfb)
diff --git a/packages/browser/src/integrations/spanstreaming.ts b/packages/browser/src/integrations/spanstreaming.ts
--- a/packages/browser/src/integrations/spanstreaming.ts
+++ b/packages/browser/src/integrations/spanstreaming.ts
@@ -1,13 +1,5 @@
import type { IntegrationFn } from '@sentry/core';
-import {
- captureSpan,
- debug,
- defineIntegration,
- hasSpanStreamingEnabled,
- isStreamedBeforeSendSpanCallback,
- SpanBuffer,
- spanIsSampled,
-} from '@sentry/core';
+import { debug, defineIntegration, setupSpanStreaming } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
export const spanStreamingIntegration = defineIntegration(() => {
@@ -25,36 +17,12 @@
},
setup(client) {
- const initialMessage = 'SpanStreaming integration requires';
- const fallbackMsg = 'Falling back to static trace lifecycle.';
+ const buffer = setupSpanStreaming(client);
- if (!hasSpanStreamingEnabled(client)) {
- DEBUG_BUILD && debug.warn(`${initialMessage} \`traceLifecycle\` to be set to "stream"! ${fallbackMsg}`);
+ if (!buffer) {
return;
}
- const beforeSendSpan = client.getOptions().beforeSendSpan;
- // If users misconfigure their SDK by opting into span streaming but
- // using an incompatible beforeSendSpan callback, we fall back to the static trace lifecycle.
- 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 => {
- // Negatively sampled spans must not be captured.
- // This happens because OTel and we create non-recording spans for negatively sampled spans
- // that go through the same life cycle as recording spans.
- if (!spanIsSampled(span)) {
- return;
- }
- buffer.add(captureSpan(span, client));
- });
-
// In addition to capturing the span, we also flush the trace when the segment
// span ends to ensure things are sent timely. We never know when the browser
// is closed, users navigate away, etc.
diff --git a/packages/browser/test/integrations/spanstreaming.test.ts b/packages/browser/test/integrations/spanstreaming.test.ts
--- a/packages/browser/test/integrations/spanstreaming.test.ts
+++ b/packages/browser/test/integrations/spanstreaming.test.ts
@@ -1,5 +1,12 @@
import * as SentryCore from '@sentry/core';
-import { debug } from '@sentry/core';
+import {
+ captureSpan,
+ debug,
+ hasSpanStreamingEnabled,
+ isStreamedBeforeSendSpanCallback,
+ spanIsSampled,
+} from '@sentry/core';
+import type { Span } from '@sentry/core';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { BrowserClient, spanStreamingIntegration } from '../../src';
import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';
@@ -20,6 +27,35 @@
return {
...original,
SpanBuffer: MockSpanBuffer,
+ setupSpanStreaming: vi.fn((client: any) => {
+ const initialMessage = 'SpanStreaming integration requires';
+ const fallbackMsg = 'Falling back to static trace lifecycle.';
+
+ if (!(original as any).hasSpanStreamingEnabled(client)) {
+ (original as any).debug.warn(`${initialMessage} \`traceLifecycle\` to be set to "stream"! ${fallbackMsg}`);
+ return undefined;
+ }
+
+ const beforeSendSpan = client.getOptions().beforeSendSpan;
+ if (beforeSendSpan && !(original as any).isStreamedBeforeSendSpanCallback(beforeSendSpan)) {
+ client.getOptions().traceLifecycle = 'static';
+ (original as any).debug.warn(
+ `${initialMessage} a beforeSendSpan callback using \`withStreamedSpan\`! ${fallbackMsg}`,
+ );
+ return undefined;
+ }
+
+ const buffer = new MockSpanBuffer(client);
+
+ client.on('afterSpanEnd', (span: Readonly<Span>) => {
+ if (!(original as any).spanIsSampled(span)) {
+ return;
+ }
+ buffer.add((original as any).captureSpan(span, client));
+ });
+
+ return buffer;
+ }),
};
});
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
--- a/packages/core/src/index.ts
+++ b/packages/core/src/index.ts
@@ -184,7 +184,7 @@
export { SpanBuffer } from './tracing/spans/spanBuffer';
export { hasSpanStreamingEnabled } from './tracing/spans/hasSpanStreamingEnabled';
-export { spanStreamingIntegration } from './integrations/spanStreaming';
+export { setupSpanStreaming, spanStreamingIntegration } from './integrations/spanStreaming';
export type { FeatureFlag } from './utils/featureFlags';
diff --git a/packages/core/src/integrations/spanStreaming.ts b/packages/core/src/integrations/spanStreaming.ts
--- a/packages/core/src/integrations/spanStreaming.ts
+++ b/packages/core/src/integrations/spanStreaming.ts
@@ -1,4 +1,6 @@
+import type { Client } from '../client';
import type { IntegrationFn } from '../types-hoist/integration';
+import type { Span } from '../types-hoist/span';
import { DEBUG_BUILD } from '../debug-build';
import { defineIntegration } from '../integration';
import { isStreamedBeforeSendSpanCallback } from '../tracing/spans/beforeSendSpan';
@@ -8,35 +10,44 @@
import { debug } from '../utils/debug-logger';
import { spanIsSampled } from '../utils/spanUtils';
+/**
+ * Shared setup logic for span streaming integrations.
+ * Returns a SpanBuffer if setup succeeds, or undefined if span streaming cannot be enabled.
+ */
+export function setupSpanStreaming(client: Client): SpanBuffer | undefined {
+ 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 undefined;
+ }
+
+ 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 undefined;
+ }
+
+ const buffer = new SpanBuffer(client);
+
+ client.on('afterSpanEnd', (span: Readonly<Span>) => {
+ if (!spanIsSampled(span)) {
+ return;
+ }
+ buffer.add(captureSpan(span, client));
+ });
+
+ return buffer;
+}
+
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));
- });
+ setupSpanStreaming(client);
},
};
}) satisfies IntegrationFn;This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
| }); | ||
| }, | ||
| }; | ||
| }) satisfies IntegrationFn; |
There was a problem hiding this comment.
Duplicate spanStreamingIntegration logic between core and browser
Low Severity
The new spanStreamingIntegration in @sentry/core substantially duplicates the existing spanStreamingIntegration in packages/browser/src/integrations/spanstreaming.ts. The setup logic — checking hasSpanStreamingEnabled, validating beforeSendSpan compatibility, creating a SpanBuffer, and subscribing to afterSpanEnd — is nearly identical. The browser version adds a beforeSetup hook and an afterSegmentSpanEnd listener, but the shared core could be extracted into a common helper to reduce maintenance burden and the risk of divergent bug fixes.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
|
Bugbot Autofix prepared fixes for 2 of the 3 issues found in the latest run.
Or push these changes by commenting: Preview (9280aa57d1)diff --git a/dev-packages/node-integration-tests/suites/tracing/span-streaming/scenario.ts b/dev-packages/node-integration-tests/suites/tracing/span-streaming/scenario.ts
new file mode 100644
--- /dev/null
+++ b/dev-packages/node-integration-tests/suites/tracing/span-streaming/scenario.ts
@@ -1,0 +1,19 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+ dsn: 'https://public@dsn.ingest.sentry.io/1337',
+ release: '1.0',
+ tracesSampleRate: 1.0,
+ traceLifecycle: 'stream',
+ integrations: [Sentry.spanStreamingIntegration()],
+ transport: loggingTransport,
+});
+
+Sentry.startSpan({ name: 'parent span' }, () => {
+ Sentry.startInactiveSpan({ name: 'child span 1' })?.end();
+ Sentry.startInactiveSpan({ name: 'child span 2' })?.end();
+ Sentry.startInactiveSpan({ name: 'child span 3' })?.end();
+});
+
+Sentry.getClient()?.flush(2000);
diff --git a/dev-packages/node-integration-tests/suites/tracing/span-streaming/test.ts b/dev-packages/node-integration-tests/suites/tracing/span-streaming/test.ts
new file mode 100644
--- /dev/null
+++ b/dev-packages/node-integration-tests/suites/tracing/span-streaming/test.ts
@@ -1,0 +1,8 @@
+import { test } from 'vitest';
+import { createRunner } from '../../../utils/runner';
+
+test('span streaming integration works without errors', async () => {
+ const runner = createRunner(__dirname, 'scenario.ts').ignore('span').ensureNoErrorOutput().start();
+
+ await runner.completed();
+});
diff --git a/packages/node-core/src/sdk/client.ts b/packages/node-core/src/sdk/client.ts
--- a/packages/node-core/src/sdk/client.ts
+++ b/packages/node-core/src/sdk/client.ts
@@ -9,7 +9,6 @@
_INTERNAL_flushLogsBuffer,
applySdkMetadata,
debug,
- hasSpanStreamingEnabled,
SDK_VERSION,
ServerRuntimeClient,
} from '@sentry/core';This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds a server-side span streaming implementation, for now scoped to POtel SDKs. However we can reuse some stuff from this PR to very easily enable span streaming on Cloudflare, Vercel Edge and other OTel-less platforms.
Main changes:
spanStreamingIntegrationto@sentry/core: This orchestrates the span streaming life cycle via the client and the span buffer. It's very similar to the already existingspanStreamingIntegrationin browser but doesn't expose some of the behaviour that we need only in browser.SentrySpanProcessorto emit the right client hooks instead of passing the span to theSpanExporter.spanStreamingIntegrationwhen users settraceLifecycle: 'stream'in their SDK init.Rest are tests and small refactors. I'll follow up with Node integration tests once this is merged to avoid bloating this PR further.
ref #17836