Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/browser-utils/src/metrics/cls.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,7 @@ import {
getRootSpan,
spanToJSON,
} from '@sentry/core';
import { browserPerformanceTimeOrigin, dropUndefinedKeys, htmlTreeAsString, logger } from '@sentry/core';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger } from '@sentry/core';
import type { SpanAttributes } from '@sentry/core';
import { DEBUG_BUILD } from '../debug-build';
import { addClsInstrumentationHandler } from './instrument';
Expand DownExpand Up@@ -95,13 +95,13 @@ function sendStandaloneClsSpan(clsValue: number, entry: LayoutShift | undefined,

const name = entry ? htmlTreeAsString(entry.sources[0]?.node) : 'Layout shift';

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are all static and none of them can be undefined

[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.browser.cls',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'ui.webvital.cls',
[SEMANTIC_ATTRIBUTE_EXCLUSIVE_TIME]: entry?.duration || 0,
// attach the pageload span id to the CLS span so that we can link them in the UI
'sentry.pageload.span_id': pageloadSpanId,
});
};

const span = startStandaloneWebVitalSpan({
name,
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/currentScopes.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,6 @@ import type { Client } from './client';
import { Scope } from './scope';
import type { TraceContext } from './types-hoist';
import { generateSpanId } from './utils-hoist';
import { dropUndefinedKeys } from './utils-hoist/object';

/**
* Get the currently active scope.
Expand DownExpand Up@@ -130,11 +129,14 @@ export function getTraceContextFromScope(scope: Scope): TraceContext {

const { traceId, parentSpanId, propagationSpanId } = propagationContext;

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
trace_id: traceId,
span_id: propagationSpanId || generateSpanId(),
parent_span_id: parentSpanId,
});
};

if (parentSpanId) {
traceContext.parent_span_id = parentSpanId;
}

return traceContext;
}
4 changes: 1 addition & 3 deletions packages/core/src/tracing/sentrySpan.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -373,9 +373,7 @@ export class SentrySpan implements Span {
sdkProcessingMetadata: {
capturedSpanScope,
capturedSpanIsolationScope,
...dropUndefinedKeys({
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
}),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this does not return any undefined stuff anyhow

},
...(source && {
transaction_info: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/utils-hoist/envelope.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -259,7 +259,7 @@ export function createEventEnvelopeHeaders(
...(sdkInfo && { sdk: sdkInfo }),
...(!!tunnel && dsn && { dsn: dsnToString(dsn) }),
...(dynamicSamplingContext && {
trace: dropUndefinedKeys({ ...dynamicSamplingContext }),
trace: dynamicSamplingContext,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we have to do this here, undefined stuff is dropped when it is JSON-stringified anyhow 🤔

}),
};
}
21 changes: 8 additions & 13 deletions packages/core/src/utils/applyScopeDataToEvent.ts
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
import type { ScopeData } from '../scope';
import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext';
import type { Breadcrumb, Event, Span } from '../types-hoist';
import { dropUndefinedKeys } from '../utils-hoist/object';
import { merge } from './merge';
import { getRootSpan, spanToJSON, spanToTraceContext } from './spanUtils';

Expand DownExpand Up@@ -113,24 +112,20 @@ export function mergeArray<Prop extends 'breadcrumbs' | 'fingerprint'>(
function applyDataToEvent(event: Event, data: ScopeData): void {
const { extra, tags, user, contexts, level, transactionName } = data;

const cleanedExtra = dropUndefinedKeys(extra);
if (Object.keys(cleanedExtra).length) {
event.extra = { ...cleanedExtra, ...event.extra };
if (Object.keys(extra).length) {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we really have to do all of this here 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it get dropped by normalize?

We should sanity check this with a test.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, JSON.stringify() will eventually def. drop this 🤔 Let's see if we have tests covering this/if we need to add this!

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you test this? 🤔 all our tests pass, and I think from an event perspective, it should not matter because it will never be serialized:

JSON.stringify({ blub: undefined })
--> '{}'

So I am not quite sure how we could test this nicely/better than we already do? 🤔

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah you're right, I think we are fine.

event.extra = { ...extra, ...event.extra };
}

const cleanedTags = dropUndefinedKeys(tags);
if (Object.keys(cleanedTags).length) {
event.tags = { ...cleanedTags, ...event.tags };
if (Object.keys(tags).length) {
event.tags = { ...tags, ...event.tags };
}

const cleanedUser = dropUndefinedKeys(user);
if (Object.keys(cleanedUser).length) {
event.user = { ...cleanedUser, ...event.user };
if (Object.keys(user).length) {
event.user = { ...user, ...event.user };
}

const cleanedContexts = dropUndefinedKeys(contexts);
if (Object.keys(cleanedContexts).length) {
event.contexts = { ...cleanedContexts, ...event.contexts };
if (Object.keys(contexts).length) {
event.contexts = { ...contexts, ...event.contexts };
}

if (level) {
Expand Down
48 changes: 48 additions & 0 deletions packages/core/test/lib/utils/applyScopeDataToEvent.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,6 +194,54 @@ describe('mergeScopeData', () => {
});

describe('applyScopeDataToEvent', () => {
it('should correctly merge nested event and scope data with undefined values', () => {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AbhiPrasad I added a test here that demonstrates the behavior!

const eventData: Event = {
user: {
name: 'John',
age: undefined,
location: 'New York',
newThing: undefined,
},
extra: {},
};

const scopeData: ScopeData = {
eventProcessors: [],
breadcrumbs: [],
user: {
name: 'John',
age: 30,
location: 'Vienna',
role: 'developer',
thing: undefined,
},
tags: {},
extra: {},
contexts: {},
attachments: [],
propagationContext: { traceId: '1', sampleRand: 0.42 },
sdkProcessingMetadata: {},
fingerprint: [],
};

applyScopeDataToEvent(eventData, scopeData);

// Verify merged data structure
expect(eventData).toEqual({
user: {
name: 'John',
age: undefined,
location: 'New York',
role: 'developer',
thing: undefined,
newThing: undefined,
},
extra: {},
breadcrumbs: undefined,
sdkProcessingMetadata: {},
});
});

it("doesn't apply scope.transactionName to transaction events", () => {
const data: ScopeData = {
eventProcessors: [],
Expand Down
22 changes: 10 additions & 12 deletions packages/opentelemetry/src/spanExporter.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -239,14 +239,14 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const sampleRate = span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;

const attributes: SpanAttributes = dropUndefinedKeys({
const attributes: SpanAttributes = {

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this and the removed invocation below was unnecessary, because we call dropUndefinedKeys on the whole transaction event further down, which recursively does this anyhow.

[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: sampleRate,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
...data,
...removeSentryAttributes(span.attributes),
});
};

const { links } = span;
const { traceId: trace_id, spanId: span_id } = span.spanContext();
Expand All@@ -260,7 +260,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve

const status = mapStatus(span);

const traceContext: TraceContext = dropUndefinedKeys({
const traceContext: TraceContext = {
parent_span_id,
span_id,
trace_id,
Expand All@@ -269,7 +269,7 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
op,
status: getStatusMessage(status), // As per protocol, span status is allowed to be undefined
links: convertSpanLinksForEnvelope(links),
});
};

const statusCode = attributes[ATTR_HTTP_RESPONSE_STATUS_CODE];
const responseContext = typeof statusCode === 'number' ? { response: { status_code: statusCode } } : undefined;
Expand All@@ -288,12 +288,10 @@ export function createTransactionForOtelSpan(span: ReadableSpan): TransactionEve
transaction: description,
type: 'transaction',
sdkProcessingMetadata: {
...dropUndefinedKeys({
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
}),
capturedSpanScope: capturedSpanScopes.scope,
capturedSpanIsolationScope: capturedSpanScopes.isolationScope,
sampleRate,
dynamicSamplingContext: getDynamicSamplingContextFromSpan(span as unknown as Span),
},
...(source && {
transaction_info: {
Expand DownExpand Up@@ -328,12 +326,12 @@ function createAndFinishSpanForOtelSpan(node: SpanNode, spans: SpanJSON[], sentS
const { attributes, startTime, endTime, parentSpanId, links } = span;

const { op, description, data, origin = 'manual' } = getSpanData(span);
const allData = dropUndefinedKeys({
const allData = {
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: origin,
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: op,
...removeSentryAttributes(attributes),
...data,
});
};

const status = mapStatus(span);

Expand Down