refactor remoteconfig into an integration

billyvg · billyvg · commit 3253a325c2cc · 2024-05-31T16:48:15.000-04:00
diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts
@@ -9,7 +9,6 @@ import type {
   EventHint,
   Options,
   ParameterizedString,
-  RemoteConfigStorage,
   SeverityLevel,
   UserFeedback,
 } from '@sentry/types';
@@ -120,38 +119,6 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
     return super._prepareEvent(event, hint, scope);
   }
 
-  /**
-   * @inheritdoc
-   */
-  protected _createRemoteStorage(): RemoteConfigStorage {
-    function getKey(key: string): string {
-      return `_sentryRC:${key}`;
-    }
-
-    return {
-      get(key: string): unknown {
-        const value = WINDOW.localStorage.getItem(getKey(key));
-
-        if (value === null) {
-          return value;
-        }
-
-        try {
-          return JSON.parse(value);
-        } catch {
-          return null;
-        }
-      },
-      set(key: string, value: any): void {
-        try {
-          WINDOW.localStorage.setItem(getKey(key), JSON.stringify(value));
-        } catch {
-          DEBUG_BUILD && logger.error('Unable to serialize configuration and save to localStorage');
-        }
-      },
-    };
-  }
-
   /**
    * Sends client reports as an envelope.
    */
diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts
@@ -1,8 +1,9 @@
 export * from './exports';
 
-export { reportingObserverIntegration } from './integrations/reportingobserver';
-export { httpClientIntegration } from './integrations/httpclient';
 export { contextLinesIntegration } from './integrations/contextlines';
+export { httpClientIntegration } from './integrations/httpclient';
+export { remoteConfigIntegration } from './integrations/remoteconfig';
+export { reportingObserverIntegration } from './integrations/reportingobserver';
 
 export {
   captureConsoleIntegration,
@@ -13,10 +14,7 @@ export {
   captureFeedback,
 } from '@sentry/core';
 
-export {
-  replayIntegration,
-  getReplay,
-} from '@sentry-internal/replay';
+export { replayIntegration, getReplay } from '@sentry-internal/replay';
 export type {
   ReplayEventType,
   ReplayEventWithTime,
@@ -34,15 +32,9 @@ export { replayCanvasIntegration } from '@sentry-internal/replay-canvas';
 import { feedbackAsyncIntegration } from './feedbackAsync';
 import { feedbackSyncIntegration } from './feedbackSync';
 export { feedbackAsyncIntegration, feedbackSyncIntegration, feedbackSyncIntegration as feedbackIntegration };
-export {
-  getFeedback,
-  sendFeedback,
-} from '@sentry-internal/feedback';
+export { getFeedback, sendFeedback } from '@sentry-internal/feedback';
 
-export {
-  defaultRequestInstrumentationOptions,
-  instrumentOutgoingRequests,
-} from './tracing/request';
+export { defaultRequestInstrumentationOptions, instrumentOutgoingRequests } from './tracing/request';
 export {
   browserTracingIntegration,
   startBrowserTracingNavigationSpan,
diff --git a/packages/browser/src/integrations/remoteconfig.ts b/packages/browser/src/integrations/remoteconfig.ts
@@ -0,0 +1,101 @@
+import { defineIntegration, getClient, remoteConfig } from '@sentry/core';
+import type { IntegrationFn, RemoteConfigStorage, RemoteOverrideableConfig } from '@sentry/types';
+import { logger } from '@sentry/utils';
+import { DEBUG_BUILD } from '../debug-build';
+import { WINDOW } from '../helpers';
+
+const INTEGRATION_NAME = 'RemoteConfig';
+
+/**
+ * Remote Configuration fetches configuration from a remote server.
+ */
+const _remoteConfigIntegration = (() => {
+  let _inst: ReturnType<typeof remoteConfig> | undefined;
+  return {
+    name: INTEGRATION_NAME,
+    setupOnce() {
+      const client = getClient();
+      if (!client) {
+        // when can this happen?
+        return;
+      }
+      _inst = remoteConfig({ client, storage: browserStorage() });
+    },
+    get api() {
+      return _inst;
+    },
+    applyUpdate() {
+      if (!_inst) {
+        return;
+      }
+
+      return _inst.applyUpdate();
+    },
+    fetch() {
+      if (!_inst) {
+        return;
+      }
+
+      return _inst.fetch();
+    },
+    fetchAndApply() {
+      if (!_inst) {
+        return;
+      }
+
+      return _inst.fetchAndApply();
+    },
+    get<T>(defaultConfig: T): T {
+      if (!_inst) {
+        return defaultConfig;
+      }
+
+      return _inst.get(defaultConfig);
+    },
+    getInternal(config: RemoteOverrideableConfig): RemoteOverrideableConfig {
+      if (!_inst) {
+        return config;
+      }
+
+      return _inst.getInternal(config);
+    },
+    getSource() {
+      if (!_inst) {
+        return 'DEFAULT';
+      }
+
+      return _inst.getSource();
+    },
+  };
+}) satisfies IntegrationFn;
+
+export const remoteConfigIntegration = defineIntegration(_remoteConfigIntegration);
+
+function _getStorageKey(key: string): string {
+  return `_sentryRC:${key}`;
+}
+
+function browserStorage(): RemoteConfigStorage {
+  return {
+    get(key: string): unknown {
+      const value = WINDOW.localStorage.getItem(_getStorageKey(key));
+
+      if (value === null) {
+        return value;
+      }
+
+      try {
+        return JSON.parse(value);
+      } catch {
+        return null;
+      }
+    },
+    set(key: string, value: any): void {
+      try {
+        WINDOW.localStorage.setItem(_getStorageKey(key), JSON.stringify(value));
+      } catch {
+        DEBUG_BUILD && logger.error('Unable to serialize configuration and save to localStorage');
+      }
+    },
+  };
+}
diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts
@@ -21,9 +21,9 @@ export function makeFetchTransport(
     pendingBodySize += requestSize;
     pendingCount++;
 
+    const method = options.method || 'POST';
     const requestOptions: RequestInit = {
-      body: request.body,
-      method: 'POST',
+      method,
       referrerPolicy: 'origin',
       headers: options.headers,
       // Outgoing requests are usually cancelled when navigating to a different page, causing a "TypeError: Failed to
@@ -41,6 +41,10 @@ export function makeFetchTransport(
       ...options.fetchOptions,
     };
 
+    if (method === 'POST') {
+      requestOptions.body = request.body;
+    }
+
     if (!nativeFetch) {
       clearCachedFetchImplementation();
       return rejectedSyncPromise('No fetch implementation available');
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -17,8 +17,6 @@ import type {
   Integration,
   Outcome,
   ParameterizedString,
-  RemoteConfigInterface,
-  RemoteConfigStorage,
   SdkMetadata,
   Session,
   SessionAggregates,
@@ -50,14 +48,13 @@ import {
   uuid4,
 } from '@sentry/utils';
 
-import { getEnvelopeEndpointWithUrlEncodedAuth, getRemoteConfigEndpoint } from './api';
+import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
 import { getIsolationScope } from './currentScopes';
 import { DEBUG_BUILD } from './debug-build';
 import { createEventEnvelope, createSessionEnvelope } from './envelope';
 import type { IntegrationIndex } from './integration';
 import { afterSetupIntegrations } from './integration';
 import { setupIntegration, setupIntegrations } from './integration';
-import { remoteConfig } from './remoteconfig';
 import type { Scope } from './scope';
 import { updateSession } from './session';
 import { getDynamicSamplingContextFromClient } from './tracing/dynamicSamplingContext';
@@ -120,9 +117,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   // eslint-disable-next-line @typescript-eslint/ban-types
   private _hooks: Record<string, Function[]>;
 
-  /** Optional remote config */
-  public readonly config?: ReturnType<typeof remoteConfig>;
-
   /**
    * Initializes this client instance.
    *
@@ -154,31 +148,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
         ...options.transportOptions,
         url,
       });
-
-      // TODO: if blockForRemoteConfig is set, client should be disabled, events
-      // should be queued before we sample (e.g. in baseclient._captureEvent check
-      // if enabled, otherwise queue?). baseclient._processEvent is where
-      // sampleRate is used. then set client as ready after remote config is loaded and flush queue
-      // TODO: create/pass transport
-
-      const storage = this._createRemoteStorage();
-      // Only initialize remote config if storage is implemented
-      if (storage) {
-        const transport = options.transport({
-          tunnel: options.tunnel,
-          recordDroppedEvent: this.recordDroppedEvent.bind(this),
-          ...options.transportOptions,
-          url: getRemoteConfigEndpoint(
-            this._dsn,
-            options.tunnel,
-            options._metadata ? options._metadata.sdk : undefined,
-          ),
-        });
-        this.config = remoteConfig({
-          transport,
-          storage,
-        });
-      }
     }
   }
 
@@ -340,34 +309,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
 
   /** @inheritdoc */
   public init(): void {
-    if (this.config) {
-      this.config.fetch();
-      // const remoteConfigFetchPromise = this.config.fetchAndApply();
-      // TODO calling _setupIntegrations async will break the world
-      // const { blockForRemoteConfig } = this._options;
-      // If `blockForRemoteConfig` is configured and there is no cached config, then
-      // if (this.config.getSource() === 'DEFAULT' && blockForRemoteConfig && blockForRemoteConfig.timeout > 0) {
-      //   new Promise(resolve => {
-      //     setTimeout(resolve, blockForRemoteConfig.timeout);
-      //     remoteConfigFetchPromise.then(resolve);
-      //   })
-      //     .then(() => this.finishInit())
-      //     .catch(() => {
-      //       // TODO: Error handling
-      //     });
-      //
-      //   return;
-      // }
-
-      // TODO these keys need to be configurable
-      const config = this.config.getInternal({
-        sampleRate: this._options.sampleRate,
-        tracesSampleRate: this._options.tracesSampleRate,
-      });
-      this._options.sampleRate = config.sampleRate;
-      this._options.tracesSampleRate = config.tracesSampleRate;
-    }
-
     if (this._isEnabled()) {
       this._setupIntegrations();
     }
@@ -906,11 +847,6 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     });
   }
 
-  /**
-   * Creates a storage interface to be used for Remote Configuration
-   */
-  protected _createRemoteStorage(): RemoteConfigStorage | void {}
-
   /**
    * @inheritDoc
    */
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -44,6 +44,7 @@ export {
 export { setAsyncContextStrategy } from './asyncContext';
 export { getMainCarrier } from './carrier';
 export { makeSession, closeSession, updateSession } from './session';
+export { remoteConfig } from './remoteconfig';
 export { SessionFlusher } from './sessionflusher';
 export { Scope } from './scope';
 export { notifyEventProcessors } from './eventProcessors';
@@ -55,11 +56,7 @@ export { createTransport } from './transports/base';
 export { makeOfflineTransport } from './transports/offline';
 export { makeMultiplexedTransport } from './transports/multiplexed';
 export { SDK_VERSION } from './version';
-export {
-  getIntegrationsToSetup,
-  addIntegration,
-  defineIntegration,
-} from './integration';
+export { getIntegrationsToSetup, addIntegration, defineIntegration } from './integration';
 export { applyScopeDataToEvent, mergeScopeData } from './utils/applyScopeDataToEvent';
 export { prepareEvent } from './utils/prepareEvent';
 export { createCheckInEnvelope } from './checkin';
diff --git a/packages/core/src/remoteconfig.ts b/packages/core/src/remoteconfig.ts
diff --git a/packages/types/src/remoteconfig.ts b/packages/types/src/remoteconfig.ts
diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts
