getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.88k stars 1.55k forks source link

withScope not creating a new scope, changes affect global scope #13428

Closed danilofuchs closed 3 weeks ago

danilofuchs commented 1 month ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.26.0

Framework Version

Koa 2.15.3, @koa/router 12.0.1

Link to Sentry event

https://salvy.sentry.io/issues/5727952368/?alert_rule_id=12365646&alert_type=issue&notification_uuid=89f5bd63-4bc1-4906-965a-ce6d89193d82&project=6779396&referrer=slack

Reproduction Example/SDK Setup

// instrument.ts

import {
  getNodeAutoInstrumentations,
  getResourceDetectors,
} from "@opentelemetry/auto-instrumentations-node";
import * as opentelemetry from "@opentelemetry/sdk-node";
import * as Sentry from "@sentry/node";
import { nodeProfilingIntegration } from "@sentry/profiling-node";

const sdk = new opentelemetry.NodeSDK({
  instrumentations: getNodeAutoInstrumentations(),
  resourceDetectors: getResourceDetectors(),
});

Sentry.init({
  dsn: "ingest.sentry.io/...",

  // Set tracesSampleRate to 1.0 to capture 100%
  // of transactions for performance monitoring.
  // We recommend adjusting this value in production
  tracesSampleRate: 0,
  profilesSampleRate: 1, // Relative to tracesSampleRate

  environment: process.env.NODE_ENV,
  enabled: process.env.NODE_ENV === "prod",

  integrations: [nodeProfilingIntegration(), Sentry.prismaIntegration()],

  skipOpenTelemetrySetup: true,

  registerEsmLoaderHooks: { exclude: [/openai/] },
});

sdk.start();

Steps to Reproduce

class ErrorTracker {
  constructor(private meta: Record<string, any> = {}) {}

  public track(
    error: Error,
    data?: Record<string, any>,
    fingerPrint?: string[]
  ) {
    const oldScope = Sentry.getCurrentScope();
    Sentry.withScope((scope) => {
      console.log("IS SAME", oldScope === scope);
      if (contextData.userId) {
        scope.setUser({
          id: contextData.userId,
          email: contextData.userEmail,
        });
      }

      scope.setContext("metadata", this.meta);
      if (data) {
        scope.setExtras(data);
      }
      scope.setTags(contextData);

      if (fingerPrint) {
        scope.setFingerprint(fingerPrint);
      }

      console.log("TAGS", scope.getScopeData().tags);
      Sentry.captureException(error);
    });
    console.log("AFTER SCOPE", Sentry.getCurrentScope().getScopeData().tags);
  }
}

If calling two times, one with tags and another without:

    const errorTracker = new ErrorTracker();

    await errorTracker.track(new Error("Healthcheck"), {
      message: "TEST",
    });
    await errorTracker.track(new Error("Healthcheck"));

The tags are applied to the second error tracking:

===> First error
IS SAME true
TAGS { message: 'TEST' }
AFTER SCOPE { message: 'TEST' }

===> Second error (tags should be reset)
IS SAME true
TAGS { message: 'TEST' }
AFTER SCOPE { message: 'TEST' }

Run the following code and see from the logs that the scope within withScope is the same

Expected Result

withScope isolates tags, context, etc

Actual Result

The scope is the same within withScope and any changes to scope data are applied to subsequent errors

andreiborza commented 1 month ago

Hi, thanks for filing this. We are currently doing a company-wide hackweek and will take a look at this issue next week. See https://github.com/getsentry/sentry-javascript/issues/13421.

danilofuchs commented 1 month ago

I have tingled with the Sentry code and was able to identify the issue occurs when using OpenTelemetry without SentryContextManager

Weirdly, Sentry.validateOpenTelemetrySetup(); only outputs the error to console if the config has debug: true. I Would expect the error to always appear if the SDK is malconfigured

danilofuchs commented 1 month ago

This is my updated instrument.ts file to send tracing information to another OpenTelemetry collector, whilst not breaking Sentry:

import {
  getNodeAutoInstrumentations,
  getResourceDetectors,
} from "@opentelemetry/auto-instrumentations-node";
import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http";
import * as opentelemetry from "@opentelemetry/sdk-node";
import * as Sentry from "@sentry/node";
import { SentryPropagator, SentrySpanProcessor } from "@sentry/opentelemetry";
import { nodeProfilingIntegration } from "@sentry/profiling-node";

const sdk = new opentelemetry.NodeSDK({
  instrumentations: getNodeAutoInstrumentations(),
  resourceDetectors: getResourceDetectors(),
  spanProcessors: [
    new SentrySpanProcessor(), // Export spans to Sentry
    new opentelemetry.tracing.BatchSpanProcessor(new OTLPTraceExporter()), // Export spans to CloudWatch OLTP sidecar
  ],
  // This conflicts with Sentry's sampling.
  // If we want to enable Sentry tracing, we must
  // use SentrySampler instead of AlwaysOnSampler
  sampler: new opentelemetry.tracing.AlwaysOnSampler(),
  textMapPropagator: new SentryPropagator(),
  contextManager: new Sentry.SentryContextManager(),
});

Sentry.init({
  dsn: "https://sentry.io/...",

  // Set tracesSampleRate to 1.0 to capture 100%
  // of transactions for performance monitoring.
  // We recommend adjusting this value in production
  tracesSampleRate: 0,
  profilesSampleRate: 1, // Relative to tracesSampleRate

  environment: process.env.NODE_ENV,
  enabled: process.env.NODE_ENV === "prod",

  integrations: [nodeProfilingIntegration(), Sentry.prismaIntegration()],

  skipOpenTelemetrySetup: true,

  registerEsmLoaderHooks: { exclude: [/openai/] },
});

Sentry.validateOpenTelemetrySetup();

sdk.start();
lforst commented 3 weeks ago

@danilofuchs thanks for the patience. The SentryContextManager is vital for the SDK to work properly. Maybe there is a world in which we can do our context managing outside of OTEL but for now it is required.

It seems to me that you could figure out this issue so I will close it, but feel free to ping us here if there are any unresolved problems!