Merge branch 'develop' into abhi-parameterize-logs

Author: AbhiPrasad

packages/browser-utils/src/instrument/xhr.ts

@@ -30,7 +30,13 @@ export function instrumentXHR(): void {
 
   // eslint-disable-next-line @typescript-eslint/unbound-method
   xhrproto.open = new Proxy(xhrproto.open, {
-    apply(originalOpen, xhrOpenThisArg: XMLHttpRequest & SentryWrappedXMLHttpRequest, xhrOpenArgArray) {
+    apply(
+      originalOpen,
+      xhrOpenThisArg: XMLHttpRequest & SentryWrappedXMLHttpRequest,
+      xhrOpenArgArray:
+        | [method: string, url: string | URL]
+        | [method: string, url: string | URL, async: boolean, username?: string | null, password?: string | null],
+    ) {
       // NOTE: If you are a Sentry user, and you are seeing this stack frame,
       //       it means the error, that was caused by your XHR call did not
       //       have a stack trace. If you are using HttpClient integration,
@@ -43,7 +49,7 @@ export function instrumentXHR(): void {
       // open() should always be called with two or more arguments
       // But to be on the safe side, we actually validate this and bail out if we don't have a method & url
       const method = isString(xhrOpenArgArray[0]) ? xhrOpenArgArray[0].toUpperCase() : undefined;
-      const url = parseUrl(xhrOpenArgArray[1]);
+      const url = parseXhrUrlArg(xhrOpenArgArray[1]);
 
       if (!method || !url) {
         return originalOpen.apply(xhrOpenThisArg, xhrOpenArgArray);
@@ -147,16 +153,23 @@ export function instrumentXHR(): void {
   });
 }
 
-function parseUrl(url: string | unknown): string | undefined {
+/**
+ * Parses the URL argument of a XHR method to a string.
+ *
+ * See: https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/open#url
+ * url: A string or any other object with a stringifier — including a URL object — that provides the URL of the resource to send the request to.
+ *
+ * @param url - The URL argument of an XHR method
+ * @returns The parsed URL string or undefined if the URL is invalid
+ */
+function parseXhrUrlArg(url: unknown): string | undefined {
   if (isString(url)) {
     return url;
   }
 
   try {
-    // url can be a string or URL
-    // but since URL is not available in IE11, we do not check for it,
-    // but simply assume it is an URL and return `toString()` from it (which returns the full URL)
-    // If that fails, we just return undefined
+    // If the passed in argument is not a string, it should have a `toString` method as a stringifier.
+    // If that fails, we just return undefined (like in IE11 where URL is not available)
     return (url as URL).toString();
   } catch {} // eslint-disable-line no-empty
 

packages/browser/src/diagnose-sdk.ts

@@ -0,0 +1,41 @@
+import { getClient } from '@sentry/core';
+
+/**
+ * A function to diagnose why the SDK might not be successfully sending data.
+ *
+ * Possible return values wrapped in a Promise:
+ * - `"no-client-active"` - There was no active client when the function was called. This possibly means that the SDK was not initialized yet.
+ * - `"sentry-unreachable"` - The Sentry SaaS servers were not reachable. This likely means that there is an ad blocker active on the page or that there are other connection issues.
+ *
+ * If the function doesn't detect an issue it resolves to `undefined`.
+ */
+export async function diagnoseSdkConnectivity(): Promise<
+  'no-client-active' | 'sentry-unreachable' | 'no-dsn-configured' | void
+> {
+  const client = getClient();
+
+  if (!client) {
+    return 'no-client-active';
+  }
+
+  if (!client.getDsn()) {
+    return 'no-dsn-configured';
+  }
+
+  try {
+    // If fetch throws, there is likely an ad blocker active or there are other connective issues.
+    await fetch(
+      // We want this to be as close as possible to an actual ingest URL so that ad blockers will actually block the request
+      // We are using the "sentry-sdks" org with id 447951 not to pollute any actual organizations.
+      'https://o447951.ingest.sentry.io/api/1337/envelope/?sentry_version=7&sentry_key=1337&sentry_client=sentry.javascript.browser%2F1.33.7',
+      {
+        body: '{}',
+        method: 'POST',
+        mode: 'cors',
+        credentials: 'omit',
+      },
+    );
+  } catch {
+    return 'sentry-unreachable';
+  }
+}

packages/browser/src/index.ts

@@ -71,3 +71,4 @@ export { launchDarklyIntegration, buildLaunchDarklyFlagUsedHandler } from './int
 export { openFeatureIntegration, OpenFeatureIntegrationHook } from './integrations/featureFlags/openfeature';
 export { unleashIntegration } from './integrations/featureFlags/unleash';
 export { statsigIntegration } from './integrations/featureFlags/statsig';
+export { diagnoseSdkConnectivity } from './diagnose-sdk';

packages/core/src/client.ts

@@ -623,12 +623,19 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   public on(hook: 'close', callback: () => void): () => void;
 
   /**
-   * A hook that is called before a log is captured
+   * A hook that is called before a log is captured. This hooks runs before `beforeSendLog` is fired.
    *
    * @returns {() => void} A function that, when executed, removes the registered callback.
    */
   public on(hook: 'beforeCaptureLog', callback: (log: Log) => void): () => void;
 
+  /**
+   * A hook that is called after a log is captured
+   *
+   * @returns {() => void} A function that, when executed, removes the registered callback.
+   */
+  public on(hook: 'afterCaptureLog', callback: (log: Log) => void): () => void;
+
   /**
    * Register a hook on this client.
    */
@@ -777,10 +784,15 @@ export abstract class Client<O extends ClientOptions = ClientOptions> {
   public emit(hook: 'close'): void;
 
   /**
-   * Emit a hook event for client before capturing a log
+   * Emit a hook event for client before capturing a log. This hooks runs before `beforeSendLog` is fired.
    */
   public emit(hook: 'beforeCaptureLog', log: Log): void;
 
+  /**
+   * Emit a hook event for client after capturing a log.
+   */
+  public emit(hook: 'afterCaptureLog', log: Log): void;
+
   /**
    * Emit a hook that was previously registered via `on()`.
    */

packages/core/src/logs/index.ts

@@ -62,18 +62,28 @@ export function logAttributeToSerializedLogAttribute(key: string, value: unknown
  * @experimental This method will experience breaking changes. This is not yet part of
  * the stable Sentry SDK API and can be changed or removed without warning.
  */
-export function _INTERNAL_captureLog(log: Log, client = getClient(), scope = getCurrentScope()): void {
+export function _INTERNAL_captureLog(beforeLog: Log, client = getClient(), scope = getCurrentScope()): void {
   if (!client) {
     DEBUG_BUILD && logger.warn('No client available to capture log.');
     return;
   }
 
   const { _experiments, release, environment } = client.getOptions();
-  if (!_experiments?.enableLogs) {
+  const { enableLogs = false, beforeSendLog } = _experiments ?? {};
+  if (!enableLogs) {
     DEBUG_BUILD && logger.warn('logging option not enabled, log will not be captured.');
     return;
   }
 
+  client.emit('beforeCaptureLog', beforeLog);
+
+  const log = beforeSendLog ? beforeSendLog(beforeLog) : beforeLog;
+  if (!log) {
+    client.recordDroppedEvent('before_send', 'log_item', 1);
+    DEBUG_BUILD && logger.warn('beforeSendLog returned null, log will not be captured.');
+    return;
+  }
+
   const [, traceContext] = _getTraceInfoFromScope(client, scope);
 
   const { level, message, attributes, severityNumber } = log;
@@ -125,7 +135,7 @@ export function _INTERNAL_captureLog(log: Log, client = getClient(), scope = get
     }
   }
 
-  client.emit('beforeCaptureLog', log);
+  client.emit('afterCaptureLog', log);
 }
 
 /**

packages/core/src/server-runtime-client.ts

@@ -57,7 +57,7 @@ export class ServerRuntimeClient<
       _INTERNAL_flushLogsBuffer(client);
     });
 
-    this.on('beforeCaptureLog', log => {
+    this.on('afterCaptureLog', log => {
       client._logWeight += estimateLogSizeInBytes(log);
 
       // We flush the logs buffer if it exceeds 0.8 MB

packages/core/src/types-hoist/options.ts

@@ -2,6 +2,7 @@ import type { CaptureContext } from '../scope';
 import type { Breadcrumb, BreadcrumbHint } from './breadcrumb';
 import type { ErrorEvent, EventHint, TransactionEvent } from './event';
 import type { Integration } from './integration';
+import type { Log } from './log';
 import type { TracesSamplerSamplingContext } from './samplingcontext';
 import type { SdkMetadata } from './sdkmetadata';
 import type { SpanJSON } from './span';
@@ -188,6 +189,17 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
      * If logs support should be enabled. Defaults to false.
      */
     enableLogs?: boolean;
+    /**
+     * An event-processing callback for logs, guaranteed to be invoked after all other log
+     * processors. This allows a log to be modified or dropped before it's sent.
+     *
+     * Note that you must return a valid log from this callback. If you do not wish to modify the log, simply return
+     * it at the end. Returning `null` will cause the log to be dropped.
+     *
+     * @param log The log generated by the SDK.
+     * @returns A new log that will be sent | null.
+     */
+    beforeSendLog?: (log: Log) => Log | null;
   };
 
   /**

packages/core/test/lib/logs/index.test.ts

@@ -7,8 +7,8 @@ import {
 } from '../../../src/logs';
 import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
 import * as loggerModule from '../../../src/utils-hoist/logger';
-import { Scope } from '../../../src';
-import { parameterize } from '../../../src/utils/parameterize';
+import { Scope, fmt } from '../../../src';
+import type { Log } from '../../../src/types-hoist/log';
 
 const PUBLIC_DSN = 'https://username@domain/123';
 
@@ -194,7 +194,7 @@ describe('_INTERNAL_captureLog', () => {
     const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableLogs: true } });
     const client = new TestClient(options);
 
-    const parameterizedMessage = parameterize`Hello ${'John'}, welcome to ${'Sentry'}`;
+    const parameterizedMessage = fmt`Hello ${'John'}, welcome to ${'Sentry'}`;
 
     _INTERNAL_captureLog({ level: 'info', message: parameterizedMessage }, client, undefined);
 
@@ -216,4 +216,94 @@ describe('_INTERNAL_captureLog', () => {
       ]),
     );
   });
+
+  it('processes logs through beforeSendLog when provided', () => {
+    const beforeSendLog = vi.fn().mockImplementation(log => ({
+      ...log,
+      message: `Modified: ${log.message}`,
+      attributes: { ...log.attributes, processed: true },
+    }));
+
+    const options = getDefaultTestClientOptions({
+      dsn: PUBLIC_DSN,
+      _experiments: { enableLogs: true, beforeSendLog },
+    });
+    const client = new TestClient(options);
+
+    _INTERNAL_captureLog(
+      {
+        level: 'info',
+        message: 'original message',
+        attributes: { original: true },
+      },
+      client,
+      undefined,
+    );
+
+    expect(beforeSendLog).toHaveBeenCalledWith({
+      level: 'info',
+      message: 'original message',
+      attributes: { original: true },
+    });
+
+    const logBuffer = _INTERNAL_getLogBuffer(client);
+    expect(logBuffer).toBeDefined();
+    expect(logBuffer?.[0]).toEqual(
+      expect.objectContaining({
+        body: {
+          stringValue: 'Modified: original message',
+        },
+        attributes: expect.arrayContaining([
+          expect.objectContaining({ key: 'processed', value: { boolValue: true } }),
+          expect.objectContaining({ key: 'original', value: { boolValue: true } }),
+        ]),
+      }),
+    );
+  });
+
+  it('drops logs when beforeSendLog returns null', () => {
+    const beforeSendLog = vi.fn().mockReturnValue(null);
+    const recordDroppedEventSpy = vi.spyOn(TestClient.prototype, 'recordDroppedEvent');
+    const loggerWarnSpy = vi.spyOn(loggerModule.logger, 'warn').mockImplementation(() => undefined);
+
+    const options = getDefaultTestClientOptions({
+      dsn: PUBLIC_DSN,
+      _experiments: { enableLogs: true, beforeSendLog },
+    });
+    const client = new TestClient(options);
+
+    _INTERNAL_captureLog(
+      {
+        level: 'info',
+        message: 'test message',
+      },
+      client,
+      undefined,
+    );
+
+    expect(beforeSendLog).toHaveBeenCalled();
+    expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'log_item', 1);
+    expect(loggerWarnSpy).toHaveBeenCalledWith('beforeSendLog returned null, log will not be captured.');
+    expect(_INTERNAL_getLogBuffer(client)).toBeUndefined();
+
+    recordDroppedEventSpy.mockRestore();
+    loggerWarnSpy.mockRestore();
+  });
+
+  it('emits beforeCaptureLog and afterCaptureLog events', () => {
+    const beforeCaptureLogSpy = vi.spyOn(TestClient.prototype, 'emit');
+    const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableLogs: true } });
+    const client = new TestClient(options);
+
+    const log: Log = {
+      level: 'info',
+      message: 'test message',
+    };
+
+    _INTERNAL_captureLog(log, client, undefined);
+
+    expect(beforeCaptureLogSpy).toHaveBeenCalledWith('beforeCaptureLog', log);
+    expect(beforeCaptureLogSpy).toHaveBeenCalledWith('afterCaptureLog', log);
+    beforeCaptureLogSpy.mockRestore();
+  });
 });