Skip to content

Commit

Permalink
Merge pull request #10759 from getsentry/fn/backport-fixes
Browse files Browse the repository at this point in the history
meta: Backport fixes to v7
  • Loading branch information
mydea committed Feb 21, 2024
2 parents a77153c + f40cba0 commit 4de88db
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 48 deletions.
10 changes: 5 additions & 5 deletions packages/core/src/tracing/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,16 @@ export class Span implements SpanInterface {

protected _traceId: string;
protected _spanId: string;
protected _parentSpanId?: string;
protected _parentSpanId?: string | undefined;
protected _sampled: boolean | undefined;
protected _name?: string;
protected _name?: string | undefined;
protected _attributes: SpanAttributes;
/** Epoch timestamp in seconds when the span started. */
protected _startTime: number;
/** Epoch timestamp in seconds when the span ended. */
protected _endTime?: number;
protected _endTime?: number | undefined;
/** Internal keeper of the status */
protected _status?: SpanStatusType | string;
protected _status?: SpanStatusType | string | undefined;

private _logMessage?: string;

Expand Down Expand Up @@ -385,7 +385,7 @@ export class Span implements SpanInterface {
*/
public startChild(
spanContext?: Pick<SpanContext, Exclude<keyof SpanContext, 'sampled' | 'traceId' | 'parentSpanId'>>,
): Span {
): SpanInterface {
const childSpan = new Span({
...spanContext,
parentSpanId: this._spanId,
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/tracing/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class Transaction extends SpanClass implements TransactionInterface {

private _contexts: Contexts;

private _trimEnd?: boolean;
private _trimEnd?: boolean | undefined;

// DO NOT yet remove this property, it is used in a hack for v7 backwards compatibility.
private _frozenDynamicSamplingContext: Readonly<Partial<DynamicSamplingContext>> | undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
isPageloadTransaction, // should wait for finish signal if it's a pageload transaction
);

if (isPageloadTransaction) {
if (isPageloadTransaction && WINDOW.document) {
WINDOW.document.addEventListener('readystatechange', () => {
if (['interactive', 'complete'].includes(WINDOW.document.readyState)) {
idleTransaction.sendAutoFinishSignal();
Expand Down Expand Up @@ -307,7 +307,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
}

let activeSpan: Span | undefined;
let startingUrl: string | undefined = WINDOW.location.href;
let startingUrl: string | undefined = WINDOW.location && WINDOW.location.href;

if (client.on) {
client.on('startNavigationSpan', (context: StartSpanOptions) => {
Expand Down Expand Up @@ -335,7 +335,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
});
}

if (options.instrumentPageLoad && client.emit) {
if (options.instrumentPageLoad && client.emit && WINDOW.location) {
const context: StartSpanOptions = {
name: WINDOW.location.pathname,
// pageload should always start at timeOrigin (and needs to be in s, not ms)
Expand All @@ -348,7 +348,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
startBrowserTracingPageLoadSpan(client, context);
}

if (options.instrumentNavigation && client.emit) {
if (options.instrumentNavigation && client.emit && WINDOW.location) {
addHistoryInstrumentationHandler(({ to, from }) => {
/**
* This early return is there to account for some cases where a navigation transaction starts right after
Expand Down
7 changes: 4 additions & 3 deletions packages/tracing/test/integrations/apollo-nestjs.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/* eslint-disable deprecation/deprecation */
/* eslint-disable @typescript-eslint/unbound-method */
import { Hub, Scope } from '@sentry/core';
import { Hub, Scope, Span as SpanClass } from '@sentry/core';
import type { Span } from '@sentry/types';
import { logger } from '@sentry/utils';

import { Integrations, Span } from '../../src';
import { Integrations } from '../../src';
import { getTestClient } from '../testutils';

type ApolloResolverGroup = {
Expand Down Expand Up @@ -79,7 +80,7 @@ describe('setupOnce', () => {

beforeEach(() => {
scope = new Scope();
parentSpan = new Span();
parentSpan = new SpanClass();
childSpan = parentSpan.startChild();
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(parentSpan);
jest.spyOn(scope, 'setSpan');
Expand Down
7 changes: 4 additions & 3 deletions packages/tracing/test/integrations/apollo.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/* eslint-disable deprecation/deprecation */
/* eslint-disable @typescript-eslint/unbound-method */
import { Hub, Scope } from '@sentry/core';
import { Hub, Scope, Span as SpanClass } from '@sentry/core';
import type { Span } from '@sentry/types';
import { logger } from '@sentry/utils';

import { Integrations, Span } from '../../src';
import { Integrations } from '../../src';
import { getTestClient } from '../testutils';

type ApolloResolverGroup = {
Expand Down Expand Up @@ -79,7 +80,7 @@ describe('setupOnce', () => {

beforeEach(() => {
scope = new Scope();
parentSpan = new Span();
parentSpan = new SpanClass();
childSpan = parentSpan.startChild();
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(parentSpan);
jest.spyOn(scope, 'setSpan');
Expand Down
7 changes: 4 additions & 3 deletions packages/tracing/test/integrations/graphql.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/* eslint-disable deprecation/deprecation */
/* eslint-disable @typescript-eslint/unbound-method */
import { Hub, Scope } from '@sentry/core';
import { Hub, Scope, Span as SpanClass } from '@sentry/core';
import type { Span } from '@sentry/types';
import { logger } from '@sentry/utils';

import { Integrations, Span } from '../../src';
import { Integrations } from '../../src';
import { getTestClient } from '../testutils';

const GQLExecute = {
Expand Down Expand Up @@ -41,7 +42,7 @@ describe('setupOnce', () => {

beforeEach(() => {
scope = new Scope();
parentSpan = new Span();
parentSpan = new SpanClass();
childSpan = parentSpan.startChild();
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(parentSpan);
jest.spyOn(scope, 'setSpan');
Expand Down
7 changes: 4 additions & 3 deletions packages/tracing/test/integrations/node/mongo.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
/* eslint-disable deprecation/deprecation */
/* eslint-disable @typescript-eslint/unbound-method */
import { Hub, Scope } from '@sentry/core';
import { Hub, Scope, Span as SpanClass } from '@sentry/core';
import type { Span } from '@sentry/types';
import { logger } from '@sentry/utils';

import { Integrations, Span } from '../../../src';
import { Integrations } from '../../../src';
import { getTestClient } from '../../testutils';

class Collection {
Expand Down Expand Up @@ -63,7 +64,7 @@ describe('patchOperation()', () => {

beforeEach(() => {
scope = new Scope();
parentSpan = new Span();
parentSpan = new SpanClass();
childSpan = parentSpan.startChild();
testClient = getTestClient({});
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(parentSpan);
Expand Down
9 changes: 5 additions & 4 deletions packages/tracing/test/integrations/node/postgres.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
/* eslint-disable deprecation/deprecation */
/* eslint-disable @typescript-eslint/unbound-method */
import { Hub, Scope } from '@sentry/core';
import { Hub, Scope, Span as SpanClass } from '@sentry/core';
import type { Span } from '@sentry/types';
import { loadModule, logger } from '@sentry/utils';
import pg from 'pg';

import { Integrations, Span } from '../../../src';
import { Integrations } from '../../../src';
import { getTestClient } from '../../testutils';

class PgClient {
Expand Down Expand Up @@ -63,7 +64,7 @@ describe('setupOnce', () => {

beforeEach(() => {
scope = new Scope();
parentSpan = new Span();
parentSpan = new SpanClass();
childSpan = parentSpan.startChild();
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(parentSpan);
jest.spyOn(parentSpan, 'startChild').mockReturnValueOnce(childSpan);
Expand Down Expand Up @@ -134,7 +135,7 @@ describe('setupOnce', () => {

it('does not attempt resolution when module is passed directly', async () => {
const scope = new Scope();
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(new Span());
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(new SpanClass());

new Integrations.Postgres({ module: mockModule }).setupOnce(
() => undefined,
Expand Down
4 changes: 2 additions & 2 deletions packages/tracing/test/span.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ describe('Span', () => {
const span2 = transaction.startChild();
const span3 = span2.startChild();
span3.end();
expect(transaction.spanRecorder).toBe(span2.spanRecorder);
expect(transaction.spanRecorder).toBe(span3.spanRecorder);
expect(transaction.spanRecorder).toBe((span2 as Span).spanRecorder);
expect(transaction.spanRecorder).toBe((span3 as Span).spanRecorder);
});
});

Expand Down
36 changes: 18 additions & 18 deletions packages/types/src/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,43 +104,43 @@ export interface SpanContext {
*
* @deprecated Use `name` instead.
*/
description?: string;
description?: string | undefined;

/**
* Human-readable identifier for the span. Alias for span.description.
*/
name?: string;
name?: string | undefined;

/**
* Operation of the Span.
*/
op?: string;
op?: string | undefined;

/**
* Completion status of the Span.
* See: {@sentry/tracing SpanStatus} for possible values
*/
status?: string;
status?: string | undefined;

/**
* Parent Span ID
*/
parentSpanId?: string;
parentSpanId?: string | undefined;

/**
* Was this span chosen to be sent as part of the sample?
*/
sampled?: boolean;
sampled?: boolean | undefined;

/**
* Span ID
*/
spanId?: string;
spanId?: string | undefined;

/**
* Trace ID
*/
traceId?: string;
traceId?: string | undefined;

/**
* Tags of the Span.
Expand All @@ -162,22 +162,22 @@ export interface SpanContext {
/**
* Timestamp in seconds (epoch time) indicating when the span started.
*/
startTimestamp?: number;
startTimestamp?: number | undefined;

/**
* Timestamp in seconds (epoch time) indicating when the span ended.
*/
endTimestamp?: number;
endTimestamp?: number | undefined;

/**
* The instrumenter that created this span.
*/
instrumenter?: Instrumenter;
instrumenter?: Instrumenter | undefined;

/**
* The origin of the span, giving context about what created the span.
*/
origin?: SpanOrigin;
origin?: SpanOrigin | undefined;
}

/** Span holding trace_id, span_id */
Expand All @@ -194,7 +194,7 @@ export interface Span extends Omit<SpanContext, 'op' | 'status' | 'origin'> {
* @deprecated Use `startSpan()` functions to set, `span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'op')
* to update and `spanToJSON().op` to read the op instead
*/
op?: string;
op?: string | undefined;

/**
* The ID of the span.
Expand All @@ -207,7 +207,7 @@ export interface Span extends Omit<SpanContext, 'op' | 'status' | 'origin'> {
*
* @deprecated Use `spanToJSON(span).parent_span_id` instead.
*/
parentSpanId?: string;
parentSpanId?: string | undefined;

/**
* The ID of the trace.
Expand All @@ -219,7 +219,7 @@ export interface Span extends Omit<SpanContext, 'op' | 'status' | 'origin'> {
* Was this span chosen to be sent as part of the sample?
* @deprecated Use `isRecording()` instead.
*/
sampled?: boolean;
sampled?: boolean | undefined;

/**
* Timestamp in seconds (epoch time) indicating when the span started.
Expand All @@ -231,7 +231,7 @@ export interface Span extends Omit<SpanContext, 'op' | 'status' | 'origin'> {
* Timestamp in seconds (epoch time) indicating when the span ended.
* @deprecated Use `spanToJSON()` instead.
*/
endTimestamp?: number;
endTimestamp?: number | undefined;

/**
* Tags for the span.
Expand Down Expand Up @@ -271,14 +271,14 @@ export interface Span extends Omit<SpanContext, 'op' | 'status' | 'origin'> {
*
* @deprecated Use `.setStatus` to set or update and `spanToJSON()` to read the status.
*/
status?: string;
status?: string | undefined;

/**
* The origin of the span, giving context about what created the span.
*
* @deprecated Use `startSpan` function to set and `spanToJSON(span).origin` to read the origin instead.
*/
origin?: SpanOrigin;
origin?: SpanOrigin | undefined;

/**
* Get context data for this span.
Expand Down
4 changes: 2 additions & 2 deletions packages/types/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ export interface TransactionContext extends SpanContext {
* accounted for in child spans, like what happens in the idle transaction Tracing integration, where we finish the
* transaction after a given "idle time" and we don't want this "idle time" to be part of the transaction.
*/
trimEnd?: boolean;
trimEnd?: boolean | undefined;

/**
* If this transaction has a parent, the parent's sampling decision
*/
parentSampled?: boolean;
parentSampled?: boolean | undefined;

/**
* Metadata associated with the transaction, for internal SDK use.
Expand Down

0 comments on commit 4de88db

Please sign in to comment.