ref: Fix circular dependency checks & setup (#12159)

I noticed we were getting some build warnings for circular dependency issues, which indicated that madge was not correctly capturing all issues. So I went ahead and updated madge to latest, and aligned the setup, which actually surfaced problems again more correctly. I also fixed all the circular dep issues that were raised there. We still have a bunch of types-related circular deps but these are hard to solve, so let's ignore this for now.
getsentry · May 22, 2024 · 53d814b · 53d814b
1 parent 772269a
commit 53d814b
Show file tree

Hide file tree

Showing 24 changed files with 338 additions and 403 deletions.
diff --git a/.madgerc b/.madgerc
@@ -0,0 +1,7 @@
+{
+  "detectiveOptions": {
+    "ts": {
+      "skipTypeImports": true
+    }
+  }
+}
diff --git a/package.json b/package.json
@@ -114,7 +114,7 @@
     "jest-environment-node": "^27.5.1",
     "jsdom": "^21.1.2",
     "lerna": "7.1.1",
-    "madge": "4.0.2",
+    "madge": "7.0.0",
     "nodemon": "^2.0.16",
     "npm-run-all": "^4.1.5",
     "prettier": "^3.1.1",

diff --git a/packages/bun/package.json b/packages/bun/package.json
@@ -76,12 +76,5 @@
   "volta": {
     "extends": "../../package.json"
   },
-  "sideEffects": false,
-  "madge": {
-    "detectiveOptions": {
-      "ts": {
-        "skipTypeImports": true
-      }
-    }
-  }
+  "sideEffects": false
 }
diff --git a/packages/core/package.json b/packages/core/package.json
@@ -69,12 +69,5 @@
   "volta": {
     "extends": "../../package.json"
   },
-  "madge": {
-    "detectiveOptions": {
-      "ts": {
-        "skipTypeImports": true
-      }
-    }
-  },
   "sideEffects": false
 }
diff --git a/packages/core/src/asyncContext/stackStrategy.ts b/packages/core/src/asyncContext/stackStrategy.ts
@@ -1,6 +1,6 @@
 import type { Client, Scope as ScopeInterface } from '@sentry/types';
 import { isThenable } from '@sentry/utils';
-import { getDefaultCurrentScope, getDefaultIsolationScope } from '../currentScopes';
+import { getDefaultCurrentScope, getDefaultIsolationScope } from '../defaultScopes';
 import { Scope } from '../scope';
 
 import { getMainCarrier, getSentryCarrier } from './../carrier';

diff --git a/packages/core/src/currentScopes.ts b/packages/core/src/currentScopes.ts
@@ -5,16 +5,6 @@ import { getAsyncContextStrategy } from './asyncContext';
 import { getMainCarrier } from './carrier';
 import { Scope as ScopeClass } from './scope';
 
-/** Get the default current scope. */
-export function getDefaultCurrentScope(): Scope {
-  return getGlobalSingleton('defaultCurrentScope', () => new ScopeClass());
-}
-
-/** Get the default isolation scope. */
-export function getDefaultIsolationScope(): Scope {
-  return getGlobalSingleton('defaultIsolationScope', () => new ScopeClass());
-}
-
 /**
  * Get the currently active scope.
  */

diff --git a/packages/core/src/defaultScopes.ts b/packages/core/src/defaultScopes.ts
@@ -0,0 +1,13 @@
+import type { Scope } from '@sentry/types';
+import { getGlobalSingleton } from '@sentry/utils';
+import { Scope as ScopeClass } from './scope';
+
+/** Get the default current scope. */
+export function getDefaultCurrentScope(): Scope {
+  return getGlobalSingleton('defaultCurrentScope', () => new ScopeClass());
+}
+
+/** Get the default isolation scope. */
+export function getDefaultIsolationScope(): Scope {
+  return getGlobalSingleton('defaultIsolationScope', () => new ScopeClass());
+}
diff --git a/packages/core/src/envelope.ts b/packages/core/src/envelope.ts
@@ -22,7 +22,8 @@ import {
   getSdkMetadataForEnvelopeHeader,
 } from '@sentry/utils';
 import { createSpanEnvelopeItem } from '@sentry/utils';
-import { type SentrySpan, getDynamicSamplingContextFromSpan } from './tracing';
+import { getDynamicSamplingContextFromSpan } from './tracing/dynamicSamplingContext';
+import type { SentrySpan } from './tracing/sentrySpan';
 import { spanToJSON } from './utils/spanUtils';
 
 /**

diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -38,9 +38,11 @@ export {
   withScope,
   withIsolationScope,
   getClient,
+} from './currentScopes';
+export {
   getDefaultCurrentScope,
   getDefaultIsolationScope,
-} from './currentScopes';
+} from './defaultScopes';
 export { setAsyncContextStrategy } from './asyncContext';
 export { getMainCarrier } from './carrier';
 export { makeSession, closeSession, updateSession } from './session';
@@ -69,7 +71,6 @@ export { handleCallbackErrors } from './utils/handleCallbackErrors';
 export { parameterize } from './utils/parameterize';
 export {
   spanToTraceHeader,
-  spanToBaggageHeader,
   spanToJSON,
   spanIsSampled,
   spanToTraceContext,

diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts
@@ -1,5 +1,9 @@
 import type { Client, DynamicSamplingContext, Span } from '@sentry/types';
-import { addNonEnumerableProperty, dropUndefinedKeys } from '@sentry/utils';
+import {
+  addNonEnumerableProperty,
+  dropUndefinedKeys,
+  dynamicSamplingContextToSentryBaggageHeader,
+} from '@sentry/utils';
 
 import { DEFAULT_ENVIRONMENT } from '../constants';
 import { getClient } from '../currentScopes';
@@ -93,3 +97,11 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly<Partial<
 
   return dsc;
 }
+
+/**
+ * Convert a Span to a baggage header.
+ */
+export function spanToBaggageHeader(span: Span): string | undefined {
+  const dsc = getDynamicSamplingContextFromSpan(span);
+  return dynamicSamplingContextToSentryBaggageHeader(dsc);
+}
diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts
@@ -18,7 +18,11 @@ export {
   withActiveSpan,
   suppressTracing,
 } from './trace';
-export { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
+export {
+  getDynamicSamplingContextFromClient,
+  getDynamicSamplingContextFromSpan,
+  spanToBaggageHeader,
+} from './dynamicSamplingContext';
 export { setMeasurement, timedEventsToMeasurements } from './measurement';
 export { sampleSpan } from './sampling';
 export { logSpanEnd, logSpanStart } from './logSpans';
diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts
@@ -12,7 +12,6 @@ import type {
 import {
   addNonEnumerableProperty,
   dropUndefinedKeys,
-  dynamicSamplingContextToSentryBaggageHeader,
   generateSentryTraceHeader,
   timestampInSeconds,
 } from '@sentry/utils';
@@ -22,7 +21,6 @@ import { getCurrentScope } from '../currentScopes';
 import { getMetricSummaryJsonForSpan, updateMetricSummaryOnSpan } from '../metrics/metric-summary';
 import type { MetricType } from '../metrics/types';
 import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../semanticAttributes';
-import { getDynamicSamplingContextFromSpan } from '../tracing';
 import type { SentrySpan } from '../tracing/sentrySpan';
 import { SPAN_STATUS_OK, SPAN_STATUS_UNSET } from '../tracing/spanstatus';
 import { _getSpanForScope } from './spanOnScope';
@@ -70,14 +68,6 @@ export function spanToTraceHeader(span: Span): string {
   return generateSentryTraceHeader(traceId, spanId, sampled);
 }
 
-/**
- * Convert a Span to a baggage header.
- */
-export function spanToBaggageHeader(span: Span): string | undefined {
-  const dsc = getDynamicSamplingContextFromSpan(span);
-  return dynamicSamplingContextToSentryBaggageHeader(dsc);
-}
-
 /**
  * Convert a span time input intp a timestamp in seconds.
  */

diff --git a/packages/deno/package.json b/packages/deno/package.json
@@ -63,13 +63,6 @@
     "extends": "../../package.json"
   },
   "sideEffects": false,
-  "madge": {
-    "detectiveOptions": {
-      "ts": {
-        "skipTypeImports": true
-      }
-    }
-  },
   "nx": {
     "targets": {
       "build:transpile": {

diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts
@@ -1,9 +1,8 @@
 import * as inspector from 'node:inspector';
 import { Worker } from 'node:worker_threads';
-import { defineIntegration, mergeScopeData } from '@sentry/core';
+import { defineIntegration, getCurrentScope, getGlobalScope, getIsolationScope, mergeScopeData } from '@sentry/core';
 import type { Contexts, Event, EventHint, Integration, IntegrationFn, ScopeData } from '@sentry/types';
 import { GLOBAL_OBJ, logger } from '@sentry/utils';
-import { getCurrentScope, getGlobalScope, getIsolationScope } from '../..';
 import { NODE_VERSION } from '../../nodeVersion';
 import type { NodeClient } from '../../sdk/client';
 import type { AnrIntegrationOptions, WorkerStartData } from './common';

diff --git a/packages/node/src/sdk/init.ts b/packages/node/src/sdk/init.ts
@@ -37,15 +37,11 @@ import { spotlightIntegration } from '../integrations/spotlight';
 import { getAutoPerformanceIntegrations } from '../integrations/tracing';
 import { makeNodeTransport } from '../transports';
 import type { NodeClientOptions, NodeOptions } from '../types';
+import { isCjs } from '../utils/commonjs';
 import { defaultStackParser, getSentryRelease } from './api';
 import { NodeClient } from './client';
 import { initOpenTelemetry } from './initOtel';
 
-/** Detect CommonJS. */
-export function isCjs(): boolean {
-  return typeof require !== 'undefined';
-}
-
 function getCjsOnlyIntegrations(): Integration[] {
   return isCjs() ? [modulesIntegration()] : [];
 }

diff --git a/packages/node/src/utils/commonjs.ts b/packages/node/src/utils/commonjs.ts
@@ -0,0 +1,4 @@
+/** Detect CommonJS. */
+export function isCjs(): boolean {
+  return typeof require !== 'undefined';
+}
diff --git a/packages/node/src/utils/ensureIsWrapped.ts b/packages/node/src/utils/ensureIsWrapped.ts
@@ -1,7 +1,7 @@
 import { isWrapped } from '@opentelemetry/core';
 import { hasTracingEnabled, isEnabled } from '@sentry/core';
 import { consoleSandbox } from '@sentry/utils';
-import { isCjs } from '../sdk/init';
+import { isCjs } from './commonjs';
 
 /**
  * Checks and warns if a framework isn't wrapped by opentelemetry.

diff --git a/packages/types/src/event.ts b/packages/types/src/event.ts
@@ -16,7 +16,7 @@ import type { Thread } from './thread';
 import type { TransactionSource } from './transaction';
 import type { User } from './user';
 
-/** JSDoc */
+/** An event to be sent to Sentry. */
 export interface Event {
   event_id?: string;
   message?: string;

diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
@@ -121,11 +121,10 @@ export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from
 export type { PropagationContext, TracePropagationTargets } from './tracing';
 export type { StartSpanOptions } from './startSpanOptions';
 export type {
-  CustomSamplingContext,
-  SamplingContext,
   TraceparentData,
   TransactionSource,
 } from './transaction';
+export type { CustomSamplingContext, SamplingContext } from './samplingcontext';
 export type {
   DurationUnit,
   InformationUnit,

diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts
@@ -1,12 +1,12 @@
 import type { Breadcrumb, BreadcrumbHint } from './breadcrumb';
 import type { ErrorEvent, EventHint, TransactionEvent } from './event';
 import type { Integration } from './integration';
+import type { SamplingContext } from './samplingcontext';
 import type { CaptureContext } from './scope';
 import type { SdkMetadata } from './sdkmetadata';
 import type { SpanJSON } from './span';
 import type { StackLineParser, StackParser } from './stacktrace';
 import type { TracePropagationTargets } from './tracing';
-import type { SamplingContext } from './transaction';
 import type { BaseTransportOptions, Transport } from './transport';
 
 export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOptions> {

diff --git a/packages/types/src/samplingcontext.ts b/packages/types/src/samplingcontext.ts
@@ -0,0 +1,47 @@
+import type { ExtractedNodeRequestData, WorkerLocation } from './misc';
+import type { SpanAttributes } from './span';
+
+/**
+ * Context data passed by the user when starting a transaction, to be used by the tracesSampler method.
+ */
+export interface CustomSamplingContext {
+  [key: string]: any;
+}
+
+/**
+ * Data passed to the `tracesSampler` function, which forms the basis for whatever decisions it might make.
+ *
+ * Adds default data to data provided by the user. See {@link Hub.startTransaction}
+ */
+export interface SamplingContext extends CustomSamplingContext {
+  /**
+   * Context data with which transaction being sampled was created.
+   * @deprecated This is duplicate data and will be removed eventually.
+   */
+  transactionContext: {
+    name: string;
+    parentSampled?: boolean | undefined;
+  };
+
+  /**
+   * Sampling decision from the parent transaction, if any.
+   */
+  parentSampled?: boolean;
+
+  /**
+   * Object representing the URL of the current page or worker script. Passed by default when using the `BrowserTracing`
+   * integration.
+   */
+  location?: WorkerLocation;
+
+  /**
+   * Object representing the incoming request to a node server. Passed by default when using the TracingHandler.
+   */
+  request?: ExtractedNodeRequestData;
+
+  /** The name of the span being sampled. */
+  name: string;
+
+  /** Initial attributes that have been passed to the span being sampled. */
+  attributes?: SpanAttributes;
+}
diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts
@@ -1,6 +1,3 @@
-import type { ExtractedNodeRequestData, WorkerLocation } from './misc';
-import type { SpanAttributes } from './span';
-
 /**
  * Data pulled from a `sentry-trace` header
  */
@@ -21,51 +18,6 @@ export interface TraceparentData {
   parentSampled?: boolean | undefined;
 }
 
-/**
- * Context data passed by the user when starting a transaction, to be used by the tracesSampler method.
- */
-export interface CustomSamplingContext {
-  [key: string]: any;
-}
-
-/**
- * Data passed to the `tracesSampler` function, which forms the basis for whatever decisions it might make.
- *
- * Adds default data to data provided by the user. See {@link Hub.startTransaction}
- */
-export interface SamplingContext extends CustomSamplingContext {
-  /**
-   * Context data with which transaction being sampled was created.
-   * @deprecated This is duplicate data and will be removed eventually.
-   */
-  transactionContext: {
-    name: string;
-    parentSampled?: boolean | undefined;
-  };
-
-  /**
-   * Sampling decision from the parent transaction, if any.
-   */
-  parentSampled?: boolean;
-
-  /**
-   * Object representing the URL of the current page or worker script. Passed by default when using the `BrowserTracing`
-   * integration.
-   */
-  location?: WorkerLocation;
-
-  /**
-   * Object representing the incoming request to a node server. Passed by default when using the TracingHandler.
-   */
-  request?: ExtractedNodeRequestData;
-
-  /** The name of the span being sampled. */
-  name: string;
-
-  /** Initial attributes that have been passed to the span being sampled. */
-  attributes?: SpanAttributes;
-}
-
 /**
  * Contains information about how the name of the transaction was determined. This will be used by the server to decide
  * whether or not to scrub identifiers from the transaction name, or replace the entire name with a placeholder.

diff --git a/packages/vercel-edge/package.json b/packages/vercel-edge/package.json
@@ -73,12 +73,5 @@
   "volta": {
     "extends": "../../package.json"
   },
-  "sideEffects": false,
-  "madge": {
-    "detectiveOptions": {
-      "ts": {
-        "skipTypeImports": true
-      }
-    }
-  }
+  "sideEffects": false
 }