From 35d1d09145fcfd8f48948e7b2dd3d9cdfbe02c59 Mon Sep 17 00:00:00 2001
From: Abhijeet Prasad <aprasad@sentry.io>
Date: Thu, 20 Mar 2025 16:22:38 -0400
Subject: [PATCH] feat(browser): Add logger.X methods to browser SDK

---
 packages/browser/src/client.ts           |   4 -
 packages/browser/src/index.ts            |   4 +
 packages/browser/src/log.ts              | 186 +++++++++++++++++++++
 packages/browser/test/index.test.ts      |  43 +++--
 packages/browser/test/log.test.ts        | 200 +++++++++++++++++++++++
 packages/core/src/index.ts               |   2 +-
 packages/core/src/logs/index.ts          |   2 +-
 packages/core/test/lib/log/index.test.ts |  20 +--
 8 files changed, 431 insertions(+), 30 deletions(-)
 create mode 100644 packages/browser/src/log.ts
 create mode 100644 packages/browser/test/log.test.ts

diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts
index bacf269871a4..0e5b3fb6214c 100644
--- a/packages/browser/src/client.ts
+++ b/packages/browser/src/client.ts
@@ -15,7 +15,6 @@ import {
   addAutoIpAddressToUser,
   applySdkMetadata,
   getSDKSource,
-  _INTERNAL_flushLogsBuffer,
 } from '@sentry/core';
 import { eventFromException, eventFromMessage } from './eventbuilder';
 import { WINDOW } from './helpers';
@@ -86,9 +85,6 @@ export class BrowserClient extends Client<BrowserClientOptions> {
       WINDOW.document.addEventListener('visibilitychange', () => {
         if (WINDOW.document.visibilityState === 'hidden') {
           this._flushOutcomes();
-          if (this._options._experiments?.enableLogs) {
-            _INTERNAL_flushLogsBuffer(this);
-          }
         }
       });
     }
diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts
index d034330b6283..47bef0cd6dae 100644
--- a/packages/browser/src/index.ts
+++ b/packages/browser/src/index.ts
@@ -1,5 +1,9 @@
 export * from './exports';
 
+import * as logger from './log';
+
+export { logger };
+
 export { reportingObserverIntegration } from './integrations/reportingobserver';
 export { httpClientIntegration } from './integrations/httpclient';
 export { contextLinesIntegration } from './integrations/contextlines';
diff --git a/packages/browser/src/log.ts b/packages/browser/src/log.ts
new file mode 100644
index 000000000000..4ae97fd27a94
--- /dev/null
+++ b/packages/browser/src/log.ts
@@ -0,0 +1,186 @@
+import type { LogSeverityLevel, Log, Client } from '@sentry/core';
+import { getClient, _INTERNAL_captureLog, _INTERNAL_flushLogsBuffer } from '@sentry/core';
+
+import { WINDOW } from './helpers';
+
+/**
+ * TODO: Make this configurable
+ */
+const DEFAULT_FLUSH_INTERVAL = 5000;
+
+let timeout: ReturnType<typeof setTimeout> | undefined;
+
+/**
+ * This is a global timeout that is used to flush the logs buffer.
+ * It is used to ensure that logs are flushed even if the client is not flushed.
+ */
+function startFlushTimeout(client: Client): void {
+  if (timeout) {
+    clearTimeout(timeout);
+  }
+
+  timeout = setTimeout(() => {
+    _INTERNAL_flushLogsBuffer(client);
+  }, DEFAULT_FLUSH_INTERVAL);
+}
+
+let isClientListenerAdded = false;
+/**
+ * This is a function that is used to add a flush listener to the client.
+ * It is used to ensure that the logger buffer is flushed when the client is flushed.
+ */
+function addFlushingListeners(client: Client): void {
+  if (isClientListenerAdded || !client.getOptions()._experiments?.enableLogs) {
+    return;
+  }
+
+  isClientListenerAdded = true;
+
+  if (WINDOW.document) {
+    WINDOW.document.addEventListener('visibilitychange', () => {
+      if (WINDOW.document.visibilityState === 'hidden') {
+        _INTERNAL_flushLogsBuffer(client);
+      }
+    });
+  }
+
+  client.on('flush', () => {
+    _INTERNAL_flushLogsBuffer(client);
+  });
+}
+
+/**
+ * Capture a log with the given level.
+ *
+ * @param level - The level of the log.
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ * @param severityNumber - The severity number of the log.
+ */
+function captureLog(
+  level: LogSeverityLevel,
+  message: string,
+  attributes?: Log['attributes'],
+  severityNumber?: Log['severityNumber'],
+): void {
+  const client = getClient();
+  if (client) {
+    addFlushingListeners(client);
+
+    startFlushTimeout(client);
+  }
+
+  _INTERNAL_captureLog({ level, message, attributes, severityNumber }, client, undefined);
+}
+
+/**
+ * @summary Capture a log with the `trace` level. Requires `_experiments.enableLogs` to be enabled.
+ *
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ *
+ * @example
+ *
+ * ```
+ * Sentry.logger.trace('Hello world', { userId: 100 });
+ * ```
+ */
+export function trace(message: string, attributes?: Log['attributes']): void {
+  captureLog('trace', message, attributes);
+}
+
+/**
+ * @summary Capture a log with the `debug` level. Requires `_experiments.enableLogs` to be enabled.
+ *
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ *
+ * @example
+ *
+ * ```
+ * Sentry.logger.debug('Hello world', { userId: 100 });
+ * ```
+ */
+export function debug(message: string, attributes?: Log['attributes']): void {
+  captureLog('debug', message, attributes);
+}
+
+/**
+ * @summary Capture a log with the `info` level. Requires `_experiments.enableLogs` to be enabled.
+ *
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ *
+ * @example
+ *
+ * ```
+ * Sentry.logger.info('Hello world', { userId: 100 });
+ * ```
+ */
+export function info(message: string, attributes?: Log['attributes']): void {
+  captureLog('info', message, attributes);
+}
+
+/**
+ * @summary Capture a log with the `warn` level. Requires `_experiments.enableLogs` to be enabled.
+ *
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ *
+ * @example
+ *
+ * ```
+ * Sentry.logger.warn('Hello world', { userId: 100 });
+ * ```
+ */
+export function warn(message: string, attributes?: Log['attributes']): void {
+  captureLog('warn', message, attributes);
+}
+
+/**
+ * @summary Capture a log with the `error` level. Requires `_experiments.enableLogs` to be enabled.
+ *
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ *
+ * @example
+ *
+ * ```
+ * Sentry.logger.error('Hello world', { userId: 100 });
+ * ```
+ */
+export function error(message: string, attributes?: Log['attributes']): void {
+  captureLog('error', message, attributes);
+}
+
+/**
+ * @summary Capture a log with the `fatal` level. Requires `_experiments.enableLogs` to be enabled.
+ *
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ *
+ * @example
+ *
+ * ```
+ * Sentry.logger.fatal('Hello world', { userId: 100 });
+ * ```
+ */
+export function fatal(message: string, attributes?: Log['attributes']): void {
+  captureLog('fatal', message, attributes);
+}
+
+/**
+ * @summary Capture a log with the `critical` level. Requires `_experiments.enableLogs` to be enabled.
+ *
+ * @param message - The message to log.
+ * @param attributes - Arbitrary structured data that stores information about the log - e.g., userId: 100.
+ *
+ * @example
+ *
+ * ```
+ * Sentry.logger.critical('Hello world', { userId: 100 });
+ * ```
+ */
+export function critical(message: string, attributes?: Log['attributes']): void {
+  captureLog('critical', message, attributes);
+}
diff --git a/packages/browser/test/index.test.ts b/packages/browser/test/index.test.ts
index 815dd39707f9..f6f66184509f 100644
--- a/packages/browser/test/index.test.ts
+++ b/packages/browser/test/index.test.ts
@@ -29,6 +29,7 @@ import {
   getCurrentScope,
   init,
   showReportDialog,
+  logger,
 } from '../src';
 import { getDefaultBrowserClientOptions } from './helper/browser-client-options';
 import { makeSimpleTransport } from './mocks/simpletransport';
@@ -243,20 +244,21 @@ describe('SentryBrowser', () => {
       expect(event.exception.values[0]?.stacktrace.frames).not.toHaveLength(0);
     });
 
-    it('should capture a message', done => {
-      const options = getDefaultBrowserClientOptions({
-        beforeSend: (event: Event): Event | null => {
-          expect(event.level).toBe('info');
-          expect(event.message).toBe('test');
-          expect(event.exception).toBeUndefined();
-          done();
-          return event;
-        },
-        dsn,
-      });
-      setCurrentClient(new BrowserClient(options));
-      captureMessage('test');
-    });
+    it('should capture an message', () =>
+      new Promise<void>(resolve => {
+        const options = getDefaultBrowserClientOptions({
+          beforeSend: event => {
+            expect(event.level).toBe('info');
+            expect(event.message).toBe('test');
+            expect(event.exception).toBeUndefined();
+            resolve();
+            return event;
+          },
+          dsn,
+        });
+        setCurrentClient(new BrowserClient(options));
+        captureMessage('test');
+      }));
 
     it('should capture an event', () =>
       new Promise<void>(resolve => {
@@ -322,6 +324,19 @@ describe('SentryBrowser', () => {
       expect(localBeforeSend).not.toHaveBeenCalled();
     });
   });
+
+  describe('logger', () => {
+    it('exports all log methods', () => {
+      expect(logger).toBeDefined();
+      expect(logger.trace).toBeDefined();
+      expect(logger.debug).toBeDefined();
+      expect(logger.info).toBeDefined();
+      expect(logger.warn).toBeDefined();
+      expect(logger.error).toBeDefined();
+      expect(logger.fatal).toBeDefined();
+      expect(logger.critical).toBeDefined();
+    });
+  });
 });
 
 describe('SentryBrowser initialization', () => {
diff --git a/packages/browser/test/log.test.ts b/packages/browser/test/log.test.ts
new file mode 100644
index 000000000000..582cc3b45d20
--- /dev/null
+++ b/packages/browser/test/log.test.ts
@@ -0,0 +1,200 @@
+/**
+ * @vitest-environment jsdom
+ */
+
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+import * as sentryCore from '@sentry/core';
+import { getGlobalScope, getCurrentScope, getIsolationScope } from '@sentry/core';
+
+import { init, logger } from '../src';
+import { makeSimpleTransport } from './mocks/simpletransport';
+
+const dsn = 'https://53039209a22b4ec1bcc296a3c9fdecd6@sentry.io/4291';
+
+// Mock the core functions
+vi.mock('@sentry/core', async requireActual => {
+  return {
+    ...((await requireActual()) as any),
+    _INTERNAL_captureLog: vi.fn(),
+    _INTERNAL_flushLogsBuffer: vi.fn(),
+  };
+});
+
+describe('Logger', () => {
+  // Use the mocked functions
+  const mockCaptureLog = vi.mocked(sentryCore._INTERNAL_captureLog);
+  const mockFlushLogsBuffer = vi.mocked(sentryCore._INTERNAL_flushLogsBuffer);
+
+  beforeEach(() => {
+    // Reset mocks
+    mockCaptureLog.mockClear();
+    mockFlushLogsBuffer.mockClear();
+
+    // Reset the global scope, isolation scope, and current scope
+    getGlobalScope().clear();
+    getIsolationScope().clear();
+    getCurrentScope().clear();
+    getCurrentScope().setClient(undefined);
+
+    // Mock setTimeout and clearTimeout
+    vi.useFakeTimers();
+
+    // Initialize with logs enabled
+    init({
+      dsn,
+      transport: makeSimpleTransport,
+      _experiments: {
+        enableLogs: true,
+      },
+    });
+  });
+
+  afterEach(() => {
+    vi.clearAllTimers();
+    vi.useRealTimers();
+  });
+
+  describe('Logger methods', () => {
+    it('should export all log methods', () => {
+      expect(logger).toBeDefined();
+      expect(logger.trace).toBeTypeOf('function');
+      expect(logger.debug).toBeTypeOf('function');
+      expect(logger.info).toBeTypeOf('function');
+      expect(logger.warn).toBeTypeOf('function');
+      expect(logger.error).toBeTypeOf('function');
+      expect(logger.fatal).toBeTypeOf('function');
+      expect(logger.critical).toBeTypeOf('function');
+    });
+
+    it('should call _INTERNAL_captureLog with trace level', () => {
+      logger.trace('Test trace message', { key: 'value' });
+      expect(mockCaptureLog).toHaveBeenCalledWith(
+        {
+          level: 'trace',
+          message: 'Test trace message',
+          attributes: { key: 'value' },
+          severityNumber: undefined,
+        },
+        expect.any(Object),
+        undefined,
+      );
+    });
+
+    it('should call _INTERNAL_captureLog with debug level', () => {
+      logger.debug('Test debug message', { key: 'value' });
+      expect(mockCaptureLog).toHaveBeenCalledWith(
+        {
+          level: 'debug',
+          message: 'Test debug message',
+          attributes: { key: 'value' },
+          severityNumber: undefined,
+        },
+        expect.any(Object),
+        undefined,
+      );
+    });
+
+    it('should call _INTERNAL_captureLog with info level', () => {
+      logger.info('Test info message', { key: 'value' });
+      expect(mockCaptureLog).toHaveBeenCalledWith(
+        {
+          level: 'info',
+          message: 'Test info message',
+          attributes: { key: 'value' },
+          severityNumber: undefined,
+        },
+        expect.any(Object),
+        undefined,
+      );
+    });
+
+    it('should call _INTERNAL_captureLog with warn level', () => {
+      logger.warn('Test warn message', { key: 'value' });
+      expect(mockCaptureLog).toHaveBeenCalledWith(
+        {
+          level: 'warn',
+          message: 'Test warn message',
+          attributes: { key: 'value' },
+          severityNumber: undefined,
+        },
+        expect.any(Object),
+        undefined,
+      );
+    });
+
+    it('should call _INTERNAL_captureLog with error level', () => {
+      logger.error('Test error message', { key: 'value' });
+      expect(mockCaptureLog).toHaveBeenCalledWith(
+        {
+          level: 'error',
+          message: 'Test error message',
+          attributes: { key: 'value' },
+          severityNumber: undefined,
+        },
+        expect.any(Object),
+        undefined,
+      );
+    });
+
+    it('should call _INTERNAL_captureLog with fatal level', () => {
+      logger.fatal('Test fatal message', { key: 'value' });
+      expect(mockCaptureLog).toHaveBeenCalledWith(
+        {
+          level: 'fatal',
+          message: 'Test fatal message',
+          attributes: { key: 'value' },
+          severityNumber: undefined,
+        },
+        expect.any(Object),
+        undefined,
+      );
+    });
+
+    it('should call _INTERNAL_captureLog with critical level', () => {
+      logger.critical('Test critical message', { key: 'value' });
+      expect(mockCaptureLog).toHaveBeenCalledWith(
+        {
+          level: 'critical',
+          message: 'Test critical message',
+          attributes: { key: 'value' },
+          severityNumber: undefined,
+        },
+        expect.any(Object),
+        undefined,
+      );
+    });
+  });
+
+  describe('Automatic flushing', () => {
+    it('should flush logs after timeout', () => {
+      logger.info('Test message');
+      expect(mockFlushLogsBuffer).not.toHaveBeenCalled();
+
+      // Fast-forward time by 5000ms (the default flush interval)
+      vi.advanceTimersByTime(5000);
+
+      expect(mockFlushLogsBuffer).toHaveBeenCalledTimes(1);
+      expect(mockFlushLogsBuffer).toHaveBeenCalledWith(expect.any(Object));
+    });
+
+    it('should restart the flush timeout when a new log is captured', () => {
+      logger.info('First message');
+
+      // Advance time by 3000ms (not enough to trigger flush)
+      vi.advanceTimersByTime(3000);
+      expect(mockFlushLogsBuffer).not.toHaveBeenCalled();
+
+      // Log another message, which should reset the timer
+      logger.info('Second message');
+
+      // Advance time by 3000ms again (should be 6000ms total, but timer was reset)
+      vi.advanceTimersByTime(3000);
+      expect(mockFlushLogsBuffer).not.toHaveBeenCalled();
+
+      // Advance time to complete the 5000ms after the second message
+      vi.advanceTimersByTime(2000);
+      expect(mockFlushLogsBuffer).toHaveBeenCalledTimes(1);
+    });
+  });
+});
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
index 53b612fccc29..292a9d5d9f6d 100644
--- a/packages/core/src/index.ts
+++ b/packages/core/src/index.ts
@@ -113,7 +113,7 @@ export { instrumentFetchRequest } from './fetch';
 export { trpcMiddleware } from './trpc';
 export { captureFeedback } from './feedback';
 export type { ReportDialogOptions } from './report-dialog';
-export { _INTERNAL_flushLogsBuffer } from './logs';
+export { _INTERNAL_captureLog, _INTERNAL_flushLogsBuffer } from './logs';
 
 // TODO: Make this structure pretty again and don't do "export *"
 export * from './utils-hoist/index';
diff --git a/packages/core/src/logs/index.ts b/packages/core/src/logs/index.ts
index dcd9cd0abaf6..6c6c9b580ee9 100644
--- a/packages/core/src/logs/index.ts
+++ b/packages/core/src/logs/index.ts
@@ -62,7 +62,7 @@ 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 captureLog(log: Log, scope = getCurrentScope(), client = getClient()): void {
+export function _INTERNAL_captureLog(log: Log, client = getClient(), scope = getCurrentScope()): void {
   if (!client) {
     DEBUG_BUILD && logger.warn('No client available to capture log.');
     return;
diff --git a/packages/core/test/lib/log/index.test.ts b/packages/core/test/lib/log/index.test.ts
index 2d975abda4e1..ab7cfe9bb4b4 100644
--- a/packages/core/test/lib/log/index.test.ts
+++ b/packages/core/test/lib/log/index.test.ts
@@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest';
 import {
   _INTERNAL_flushLogsBuffer,
   _INTERNAL_getLogBuffer,
-  captureLog,
+  _INTERNAL_captureLog,
   logAttributeToSerializedLogAttribute,
 } from '../../../src/logs';
 import { TestClient, getDefaultTestClientOptions } from '../../mocks/client';
@@ -71,12 +71,12 @@ describe('logAttributeToSerializedLogAttribute', () => {
   });
 });
 
-describe('captureLog', () => {
+describe('_INTERNAL_captureLog', () => {
   it('captures and sends logs', () => {
     const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableLogs: true } });
     const client = new TestClient(options);
 
-    captureLog({ level: 'info', message: 'test log message' }, undefined, client);
+    _INTERNAL_captureLog({ level: 'info', message: 'test log message' }, client, undefined);
     expect(_INTERNAL_getLogBuffer(client)).toHaveLength(1);
     expect(_INTERNAL_getLogBuffer(client)?.[0]).toEqual(
       expect.objectContaining({
@@ -94,7 +94,7 @@ describe('captureLog', () => {
     const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN });
     const client = new TestClient(options);
 
-    captureLog({ level: 'info', message: 'test log message' }, undefined, client);
+    _INTERNAL_captureLog({ level: 'info', message: 'test log message' }, client, undefined);
 
     expect(logWarnSpy).toHaveBeenCalledWith('logging option not enabled, log will not be captured.');
     expect(_INTERNAL_getLogBuffer(client)).toBeUndefined();
@@ -111,7 +111,7 @@ describe('captureLog', () => {
       sampleRand: 1,
     });
 
-    captureLog({ level: 'error', message: 'test log with trace' }, scope, client);
+    _INTERNAL_captureLog({ level: 'error', message: 'test log with trace' }, client, scope);
 
     expect(_INTERNAL_getLogBuffer(client)?.[0]).toEqual(
       expect.objectContaining({
@@ -129,7 +129,7 @@ describe('captureLog', () => {
     });
     const client = new TestClient(options);
 
-    captureLog({ level: 'info', message: 'test log with metadata' }, undefined, client);
+    _INTERNAL_captureLog({ level: 'info', message: 'test log with metadata' }, client, undefined);
 
     const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes;
     expect(logAttributes).toEqual(
@@ -144,14 +144,14 @@ describe('captureLog', () => {
     const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, _experiments: { enableLogs: true } });
     const client = new TestClient(options);
 
-    captureLog(
+    _INTERNAL_captureLog(
       {
         level: 'info',
         message: 'test log with custom attributes',
         attributes: { userId: '123', component: 'auth' },
       },
-      undefined,
       client,
+      undefined,
     );
 
     const logAttributes = _INTERNAL_getLogBuffer(client)?.[0]?.attributes;
@@ -169,13 +169,13 @@ describe('captureLog', () => {
 
     // Fill the buffer to max size (100 is the MAX_LOG_BUFFER_SIZE constant in client.ts)
     for (let i = 0; i < 100; i++) {
-      captureLog({ level: 'info', message: `log message ${i}` }, undefined, client);
+      _INTERNAL_captureLog({ level: 'info', message: `log message ${i}` }, client, undefined);
     }
 
     expect(_INTERNAL_getLogBuffer(client)).toHaveLength(100);
 
     // Add one more to trigger flush
-    captureLog({ level: 'info', message: 'trigger flush' }, undefined, client);
+    _INTERNAL_captureLog({ level: 'info', message: 'trigger flush' }, client, undefined);
 
     expect(_INTERNAL_getLogBuffer(client)).toEqual([]);
   });