fix(stdlib,server): correct Sentry sampled checks and dropping of trace events

Author: dirkdev98

packages/server/src/middleware/log.js

@@ -111,10 +111,8 @@ export function logMiddleware(app, options) {
 
       if (span) {
         if (!isMatchedRoute) {
-          // @ts-expect-error Private property?
-          //
           // Discard sampled spans which don't match a route.
-          span._sampled = false;
+          span.setAttribute("_compas.skip-event", true);
         }
 
         span.setStatus(

packages/stdlib/src/logger.js

@@ -3,7 +3,7 @@ import { environment, isProduction } from "./env.js";
 import { AppError } from "./error.js";
 import { isNil, isPlainObject, merge } from "./lodash.js";
 import { loggerWriteGithubActions, loggerWritePretty } from "./log-writers.js";
-import { _compasSentryExport } from "./sentry.js";
+import { _compasSentryExport, sentrySpanIsSampled } from "./sentry.js";
 
 /**
  * @typedef {object} Logger
@@ -131,13 +131,14 @@ export function newLogger(options) {
       info: (message) => {
         childLogger.info({ message });
 
-        if (!_compasSentryExport?.getActiveSpan?.()) {
-          // Don't add breadcrumbs if we don't have a span. This prevents unmatched logs from showing up in a random span.
+        if (!sentrySpanIsSampled(_compasSentryExport?.getActiveSpan?.())) {
+          // Don't add breadcrumbs if we don't have a span. This prevents unmatched logs
+          // from showing up in a random span.
           return;
         }
 
         if (!addedContextAsBreadcrumb) {
-          _compasSentryExport.addBreadcrumb({
+          _compasSentryExport?.addBreadcrumb({
             category: context.type,
             data: {
               ...context,
@@ -148,7 +149,7 @@ export function newLogger(options) {
           addedContextAsBreadcrumb = true;
         }
 
-        _compasSentryExport.addBreadcrumb({
+        _compasSentryExport?.addBreadcrumb({
           category: context.type,
           data: typeof message === "string" ? undefined : message,
           message: typeof message === "string" ? message : undefined,
@@ -159,13 +160,14 @@ export function newLogger(options) {
       error: (message) => {
         childLogger.error({ message });
 
-        if (!_compasSentryExport?.getActiveSpan?.()) {
-          // Don't add breadcrumbs if we don't have a span. This prevents unmatched logs from showing up in a random span.
+        if (!sentrySpanIsSampled(_compasSentryExport?.getActiveSpan?.())) {
+          // Don't add breadcrumbs if we don't have a span. This prevents unmatched logs
+          // from showing up in a random span.
           return;
         }
 
         if (!addedContextAsBreadcrumb) {
-          _compasSentryExport.addBreadcrumb({
+          _compasSentryExport?.addBreadcrumb({
             category: "log",
             data: {
               ...context,
@@ -176,7 +178,7 @@ export function newLogger(options) {
           addedContextAsBreadcrumb = true;
         }
 
-        _compasSentryExport.addBreadcrumb({
+        _compasSentryExport?.addBreadcrumb({
           category: "log",
           data: typeof message === "string" ? undefined : message,
           message: typeof message === "string" ? message : undefined,

packages/stdlib/src/sentry.js

@@ -1,7 +1,7 @@
 /**
  * The Sentry version that all Compas packages use if set via {@link compasWithSentry}.
  *
- * @type {undefined|import("@sentry/node")}
+ * @type {undefined|typeof import("@sentry/node")}
  */
 export let _compasSentryExport = undefined;
 
@@ -16,35 +16,61 @@ export let _compasSentryEnableQuerySpans = false;
  * Enable Sentry support. This comes with the following changes:
  *
  * Stdlib:
- * - Logger: both info and error logs are added as breadcrumbs to the current active span.
+ * - Logger: both info and error logs are added as breadcrumbs to the current active
+ * span.
  * - Event: Events are propagated to Sentry as (inactive) spans.
  *     Meaning that further logs are not necessarily correlated to the correct event.
  *     The final event callstack is not logged.
  *
  * Server:
  * - Starts a new root span for each incoming request.
  * - Tries to name it based on the finalized name of `ctx.event`.
- *     This is most likely in the format `router.foo.bar` for matched routes by the generated router.
+ *     This is most likely in the format `router.foo.bar` for matched routes by the
+ * generated router.
  * - Uses the sentry-trace header when provided.
  *     Note that if a custom list of `allowHeaders` is provided in the CORS options,
  *     'sentry-trace' and 'baggage' should be allowed as well.
- * - If the error handler retrieves an unknown or AppError.serverError, it is reported as an uncaught exception.
- *     It is advised to set 'normalizeDepth' to '0' in your Sentry config, and to enable the 'extraErrorDataIntegration' integration.
+ * - If the error handler retrieves an unknown or AppError.serverError, it is reported as
+ * an uncaught exception. It is advised to set 'normalizeDepth' to '0' in your Sentry
+ * config, and to enable the 'extraErrorDataIntegration' integration.
  *
  * Store:
  * - Starts a new root span for each handled Job in the QueueWorker
- *     The span name is based on the job name. Unhandled errors are captured as exceptions.
- * - Supports passing queries to Sentry as spans. Requires {@link opts.sendQueriesAsSpans} to be set.
+ *     The span name is based on the job name. Unhandled errors are captured as
+ * exceptions.
+ * - Supports passing queries to Sentry as spans. Requires {@link
+ * opts.sendQueriesAsSpans} to be set.
  *
  * All:
  * - All error logs in Compas package code are captured as exceptions.
  *
- * @param {import("@sentry/node")} instance
+ * @param {typeof import("@sentry/node")} instance
  * @param {{
  *   sendQueriesAsSpans?: boolean
  * }} [opts]
  */
 export function compasWithSentry(instance, { sendQueriesAsSpans } = {}) {
   _compasSentryExport = instance;
   _compasSentryEnableQuerySpans = sendQueriesAsSpans ?? false;
+
+  _compasSentryExport.addEventProcessor((event) => {
+    if (event.spans?.some?.((it) => it.data?.["_compas.skip-event"])) {
+      return null;
+    }
+
+    return event;
+  });
+}
+
+/**
+ * @see https://github.com/getsentry/sentry-javascript/blob/8bec42e0285ee301e8fc9bcaf02046daf48e0495/packages/core/src/utils/spanUtils.ts#L103
+ */
+export function sentrySpanIsSampled(span) {
+  if (!span) {
+    return false;
+  }
+
+  const { traceFlags } = span.spanContext();
+
+  return Boolean(traceFlags & 0x1);
 }