Skip to content

feat(v8): Remove data from SentrySpan & rename types #11303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 27, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ const chicken = {};
const egg = { contains: chicken };
chicken.lays = egg;

Sentry.startSpan({ name: 'circular_object_test_transaction', data: { chicken } }, () => {
Sentry.startSpan({ op: 'circular_object_test_span', data: { chicken } }, () => undefined);
Sentry.startSpan({ name: 'circular_object_test_transaction', attributes: { chicken } }, () => {
Sentry.startSpan({ op: 'circular_object_test_span', attributes: { chicken } }, () => undefined);
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { Route } from '@playwright/test';
import { expect } from '@playwright/test';
import type { Event, SpanContext, SpanJSON } from '@sentry/types';
import type { Contexts, Event, SpanJSON } from '@sentry/types';

import { sentryTest } from '../../../../utils/fixtures';
import {
Expand All @@ -11,7 +11,7 @@ import {

type TransactionJSON = SpanJSON & {
spans: SpanJSON[];
contexts: SpanContext;
contexts: Contexts;
platform: string;
type: string;
};
Expand Down
4 changes: 2 additions & 2 deletions docs/v8-new-performance-apis.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ There are three key APIs available to start spans:
- `startSpanManual()`
- `startInactiveSpan()`

All three span APIs take a `SpanContext` as a first argument, which has the following shape:
All three span APIs take `StartSpanOptions` as a first argument, which has the following shape:

```ts
interface SpanContext {
interface StartSpanOptions {
// The only required field - the name of the span
name: string;
attributes?: SpanAttributes;
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tracing/sampling.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Options, SamplingContext, TransactionContext } from '@sentry/types';
import type { Options, SamplingContext, TransactionArguments } from '@sentry/types';
import { isNaN, logger } from '@sentry/utils';

import { DEBUG_BUILD } from '../debug-build';
Expand All @@ -14,7 +14,7 @@ import { hasTracingEnabled } from '../utils/hasTracingEnabled';
* It returns the same transaction, for convenience.
*/
export function sampleTransaction(
transactionContext: TransactionContext,
transactionContext: TransactionArguments,
options: Pick<Options, 'tracesSampleRate' | 'tracesSampler' | 'enableTracing'>,
samplingContext: SamplingContext,
): [sampled: boolean, sampleRate?: number] {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tracing/sentryNonRecordingSpan.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type {
SentrySpanArguments,
Span,
SpanAttributeValue,
SpanAttributes,
SpanContext,
SpanContextData,
SpanStatus,
SpanTimeInput,
Expand All @@ -17,7 +17,7 @@ export class SentryNonRecordingSpan implements Span {
private _traceId: string;
private _spanId: string;

public constructor(spanContext: SpanContext = {}) {
public constructor(spanContext: SentrySpanArguments = {}) {
this._traceId = spanContext.traceId || uuid4();
this._spanId = spanContext.spanId || uuid4().substring(16);
}
Expand Down
68 changes: 9 additions & 59 deletions packages/core/src/tracing/sentrySpan.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type {
SentrySpanArguments,
Span,
SpanAttributeValue,
SpanAttributes,
SpanContext,
SpanContextData,
SpanJSON,
SpanOrigin,
Expand Down Expand Up @@ -32,13 +32,6 @@ import {
* Span contains all data about a span
*/
export class SentrySpan implements Span {
/**
* Data for the span.
* @deprecated Use `spanToJSON(span).atttributes` instead.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public data: { [key: string]: any };

/**
* @inheritDoc
* @deprecated Use top level `Sentry.getRootSpan()` instead
Expand Down Expand Up @@ -66,12 +59,10 @@ export class SentrySpan implements Span {
* @hideconstructor
* @hidden
*/
public constructor(spanContext: SpanContext = {}) {
public constructor(spanContext: SentrySpanArguments = {}) {
this._traceId = spanContext.traceId || uuid4();
this._spanId = spanContext.spanId || uuid4().substring(16);
this._startTime = spanContext.startTimestamp || timestampInSeconds();
// eslint-disable-next-line deprecation/deprecation
this.data = spanContext.data ? { ...spanContext.data } : {};

this._attributes = {};
this.setAttributes({
Expand Down Expand Up @@ -164,7 +155,10 @@ export class SentrySpan implements Span {
* @deprecated Use `startSpan()`, `startSpanManual()` or `startInactiveSpan()` instead.
*/
public startChild(
spanContext: Pick<SpanContext, Exclude<keyof SpanContext, 'sampled' | 'traceId' | 'parentSpanId'>> = {},
spanContext: Pick<
SentrySpanArguments,
Exclude<keyof SentrySpanArguments, 'sampled' | 'traceId' | 'parentSpanId'>
> = {},
): Span {
const childSpan = new SentrySpan({
...spanContext,
Expand Down Expand Up @@ -206,19 +200,6 @@ export class SentrySpan implements Span {
return childSpan;
}

/**
* Sets the data attribute on the current span
* @param key Data key
* @param value Data value
* @deprecated Use `setAttribute()` instead.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public setData(key: string, value: any): this {
// eslint-disable-next-line deprecation/deprecation
this.data = { ...this.data, [key]: value };
return this;
}

/** @inheritdoc */
public setAttribute(key: string, value: SpanAttributeValue | undefined): void {
if (value === undefined) {
Expand Down Expand Up @@ -291,9 +272,9 @@ export class SentrySpan implements Span {
*
* @deprecated Use `spanToJSON()` or access the fields directly instead.
*/
public toContext(): SpanContext {
public toContext(): SentrySpanArguments {
return dropUndefinedKeys({
data: this._getData(),
data: this._attributes,
name: this._name,
endTimestamp: this._endTime,
op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP],
Expand Down Expand Up @@ -325,7 +306,7 @@ export class SentrySpan implements Span {
*/
public getSpanJSON(): SpanJSON {
return dropUndefinedKeys({
data: this._getData(),
data: this._attributes,
description: this._name,
op: this._attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP],
parent_span_id: this._parentSpanId,
Expand All @@ -352,37 +333,6 @@ export class SentrySpan implements Span {
return this.getSpanJSON();
}

/**
* Get the merged data for this span.
* For now, this combines `data` and `attributes` together,
* until eventually we can ingest `attributes` directly.
*/
private _getData():
| {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[key: string]: any;
}
| undefined {
// eslint-disable-next-line deprecation/deprecation
const { data, _attributes: attributes } = this;

const hasData = Object.keys(data).length > 0;
const hasAttributes = Object.keys(attributes).length > 0;

if (!hasData && !hasAttributes) {
return undefined;
}

if (hasData && hasAttributes) {
return {
...data,
...attributes,
};
}

return hasData ? data : attributes;
}

/** Emit `spanEnd` when the span is ended. */
private _onSpanEnded(): void {
const client = getClient();
Expand Down
18 changes: 7 additions & 11 deletions packages/core/src/tracing/trace.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ClientOptions, Scope, Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types';
import type { ClientOptions, Scope, Span, SpanTimeInput, StartSpanOptions, TransactionArguments } from '@sentry/types';

import { propagationContextFromHeaders } from '@sentry/utils';
import type { AsyncContextStrategy } from '../asyncContext';
Expand Down Expand Up @@ -211,7 +211,7 @@ function createChildSpanOrTransaction({
scope,
}: {
parentSpan: SentrySpan | undefined;
spanContext: TransactionContext;
spanContext: TransactionArguments;
forceTransaction?: boolean;
scope: Scope;
}): Span {
Expand Down Expand Up @@ -275,15 +275,15 @@ function createChildSpanOrTransaction({
}

/**
* This converts StartSpanOptions to TransactionContext.
* This converts StartSpanOptions to TransactionArguments.
* For the most part (for now) we accept the same options,
* but some of them need to be transformed.
*
* Eventually the StartSpanOptions will be more aligned with OpenTelemetry.
*/
function normalizeContext(context: StartSpanOptions): TransactionContext {
function normalizeContext(context: StartSpanOptions): TransactionArguments {
if (context.startTime) {
const ctx: TransactionContext & { startTime?: SpanTimeInput } = { ...context };
const ctx: TransactionArguments & { startTime?: SpanTimeInput } = { ...context };
ctx.startTimestamp = spanTimeInputToSeconds(context.startTime);
delete ctx.startTime;
return ctx;
Expand All @@ -297,19 +297,15 @@ function getAcs(): AsyncContextStrategy {
return getAsyncContextStrategy(carrier);
}

function _startTransaction(transactionContext: TransactionContext): Transaction {
function _startTransaction(transactionContext: TransactionArguments): Transaction {
const client = getClient();
const options: Partial<ClientOptions> = (client && client.getOptions()) || {};

const [sampled, sampleRate] = sampleTransaction(transactionContext, options, {
name: transactionContext.name,
parentSampled: transactionContext.parentSampled,
transactionContext,
attributes: {
// eslint-disable-next-line deprecation/deprecation
...transactionContext.data,
...transactionContext.attributes,
},
attributes: transactionContext.attributes,
});

// eslint-disable-next-line deprecation/deprecation
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/tracing/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type {
SpanJSON,
SpanTimeInput,
Transaction as TransactionInterface,
TransactionContext,
TransactionArguments,
TransactionEvent,
TransactionMetadata,
TransactionSource,
Expand Down Expand Up @@ -52,7 +52,7 @@ export class Transaction extends SentrySpan implements TransactionInterface {
*
* @deprecated Transactions will be removed in v8. Use spans instead.
*/
public constructor(transactionContext: TransactionContext, hub?: Hub) {
public constructor(transactionContext: TransactionArguments, hub?: Hub) {
super(transactionContext);
this._measurements = {};
this._contexts = {};
Expand Down Expand Up @@ -170,7 +170,7 @@ export class Transaction extends SentrySpan implements TransactionInterface {
/**
* @inheritDoc
*/
public toContext(): TransactionContext {
public toContext(): TransactionArguments {
// eslint-disable-next-line deprecation/deprecation
const spanContext = super.toContext();

Expand Down
64 changes: 0 additions & 64 deletions packages/core/test/lib/tracing/sentrySpan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,68 +351,4 @@ describe('SentrySpan', () => {
});
});
});

// Ensure that attributes & data are merged together
describe('_getData', () => {
it('works without data & attributes', () => {
const span = new SentrySpan();

expect(span['_getData']()).toEqual({
// origin is set by default to 'manual' in the SentrySpan constructor
'sentry.origin': 'manual',
});
});

it('works with data only', () => {
const span = new SentrySpan();
// eslint-disable-next-line deprecation/deprecation
span.setData('foo', 'bar');

expect(span['_getData']()).toEqual({
foo: 'bar',
// origin is set by default to 'manual' in the SentrySpan constructor
'sentry.origin': 'manual',
});
expect(span['_getData']()).toStrictEqual({
// eslint-disable-next-line deprecation/deprecation
...span.data,
'sentry.origin': 'manual',
});
});

it('works with attributes only', () => {
const span = new SentrySpan();
span.setAttribute('foo', 'bar');

expect(span['_getData']()).toEqual({
foo: 'bar',
// origin is set by default to 'manual' in the SentrySpan constructor
'sentry.origin': 'manual',
});
// eslint-disable-next-line deprecation/deprecation
expect(span['_getData']()).toBe(span.attributes);
});

it('merges data & attributes', () => {
const span = new SentrySpan();
span.setAttribute('foo', 'foo');
span.setAttribute('bar', 'bar');
// eslint-disable-next-line deprecation/deprecation
span.setData('foo', 'foo2');
// eslint-disable-next-line deprecation/deprecation
span.setData('baz', 'baz');

expect(span['_getData']()).toEqual({
foo: 'foo',
bar: 'bar',
baz: 'baz',
// origin is set by default to 'manual' in the SentrySpan constructor
'sentry.origin': 'manual',
});
// eslint-disable-next-line deprecation/deprecation
expect(span['_getData']()).not.toBe(span.attributes);
// eslint-disable-next-line deprecation/deprecation
expect(span['_getData']()).not.toBe(span.data);
});
});
});
4 changes: 2 additions & 2 deletions packages/node/test/integrations/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { getCurrentHub, getIsolationScope, setCurrentClient } from '@sentry/core
import { Transaction } from '@sentry/core';
import { getCurrentScope, setUser, spanToJSON, startInactiveSpan } from '@sentry/core';
import { addTracingExtensions } from '@sentry/core';
import type { TransactionContext } from '@sentry/types';
import type { TransactionArguments } from '@sentry/types';
import { TRACEPARENT_REGEXP } from '@sentry/utils';
import * as nock from 'nock';
import { HttpsProxyAgent } from '../../src/proxy';
Expand Down Expand Up @@ -38,7 +38,7 @@ describe('tracing', () => {

function createTransactionOnScope(
customOptions: Partial<NodeClientOptions> = {},
customContext?: Partial<TransactionContext>,
customContext?: Partial<TransactionArguments>,
) {
setupMockHub(customOptions);
addTracingExtensions();
Expand Down
6 changes: 3 additions & 3 deletions packages/react/test/profiler.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { SentrySpan } from '@sentry/core';
import type { SpanContext } from '@sentry/types';
import type { StartSpanOptions } from '@sentry/types';
import { render } from '@testing-library/react';
import { renderHook } from '@testing-library/react-hooks';
// biome-ignore lint/nursery/noUnusedImports: Need React import for JSX
Expand All @@ -8,7 +8,7 @@ import * as React from 'react';
import { REACT_MOUNT_OP, REACT_RENDER_OP, REACT_UPDATE_OP } from '../src/constants';
import { UNKNOWN_COMPONENT, useProfiler, withProfiler } from '../src/profiler';

const mockStartInactiveSpan = jest.fn((spanArgs: SpanContext) => ({ ...spanArgs }));
const mockStartInactiveSpan = jest.fn((spanArgs: StartSpanOptions) => ({ ...spanArgs }));
const mockFinish = jest.fn();

class MockSpan extends SentrySpan {
Expand All @@ -22,7 +22,7 @@ let activeSpan: Record<string, any>;
jest.mock('@sentry/browser', () => ({
...jest.requireActual('@sentry/browser'),
getActiveSpan: () => activeSpan,
startInactiveSpan: (ctx: SpanContext) => {
startInactiveSpan: (ctx: StartSpanOptions) => {
mockStartInactiveSpan(ctx);
return new MockSpan(ctx);
},
Expand Down
Loading