feat: Send transactions in envelopes (getsentry#2553)

rhcarvalho · kamilogorek · web-flow · commit 8958f61ca666 · 2020-05-11T13:35:24.000+02:00
The new Envelope endpoint and format is what we want to use for
transactions going forward.

Also unify how `@sentry/browser` and `@sentry/node` send
requests to Sentry.

Co-authored-by: Kamil Ogórek &lt;kamil.ogorek@gmail.com&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
 
+- [core] feat: Send transactions in envelopes (#2553)
+
 ## 5.15.5
 
 - [browser/node] Add missing `BreadcrumbHint` and `EventHint` types exports (#2545)
diff --git a/packages/browser/src/integrations/breadcrumbs.ts b/packages/browser/src/integrations/breadcrumbs.ts
@@ -170,7 +170,7 @@ export class Breadcrumbs implements Integration {
     const client = getCurrentHub().getClient<BrowserClient>();
     const dsn = client && client.getDsn();
     if (this._options.sentry && dsn) {
-      const filterUrl = new API(dsn).getStoreEndpoint();
+      const filterUrl = new API(dsn).getBaseApiEndpoint();
       // if Sentry key appears in URL, don't capture it as a request
       // but rather as our own 'sentry' type breadcrumb
       if (
diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts
@@ -5,15 +5,20 @@ import { PromiseBuffer, SentryError } from '@sentry/utils';
 /** Base Transport class implementation */
 export abstract class BaseTransport implements Transport {
   /**
-   * @inheritDoc
+   * @deprecated
    */
   public url: string;
 
+  /** Helper to get Sentry API endpoints. */
+  protected readonly _api: API;
+
   /** A simple buffer holding all requests. */
   protected readonly _buffer: PromiseBuffer<Response> = new PromiseBuffer(30);
 
   public constructor(public options: TransportOptions) {
-    this.url = new API(this.options.dsn).getStoreEndpointWithUrlEncodedAuth();
+    this._api = new API(this.options.dsn);
+    // tslint:disable-next-line:deprecation
+    this.url = this._api.getStoreEndpointWithUrlEncodedAuth();
   }
 
   /**
diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts
@@ -1,3 +1,4 @@
+import { eventToSentryRequest } from '@sentry/core';
 import { Event, Response, Status } from '@sentry/types';
 import { getGlobalObject, logger, parseRetryAfterHeader, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';
 
@@ -22,8 +23,10 @@ export class FetchTransport extends BaseTransport {
       });
     }
 
-    const defaultOptions: RequestInit = {
-      body: JSON.stringify(event),
+    const sentryReq = eventToSentryRequest(event, this._api);
+
+    const options: RequestInit = {
+      body: sentryReq.body,
       method: 'POST',
       // Despite all stars in the sky saying that Edge supports old draft syntax, aka 'never', 'always', 'origin' and 'default
       // https://caniuse.com/#feat=referrer-policy
@@ -33,13 +36,13 @@ export class FetchTransport extends BaseTransport {
     };
 
     if (this.options.headers !== undefined) {
-      defaultOptions.headers = this.options.headers;
+      options.headers = this.options.headers;
     }
 
     return this._buffer.add(
       new SyncPromise<Response>((resolve, reject) => {
         global
-          .fetch(this.url, defaultOptions)
+          .fetch(sentryReq.url, options)
           .then(response => {
             const status = Status.fromHttpCode(response.status);
 
diff --git a/packages/browser/src/transports/xhr.ts b/packages/browser/src/transports/xhr.ts
@@ -1,3 +1,4 @@
+import { eventToSentryRequest } from '@sentry/core';
 import { Event, Response, Status } from '@sentry/types';
 import { logger, parseRetryAfterHeader, SyncPromise } from '@sentry/utils';
 
@@ -20,6 +21,8 @@ export class XHRTransport extends BaseTransport {
       });
     }
 
+    const sentryReq = eventToSentryRequest(event, this._api);
+
     return this._buffer.add(
       new SyncPromise<Response>((resolve, reject) => {
         const request = new XMLHttpRequest();
@@ -45,13 +48,13 @@ export class XHRTransport extends BaseTransport {
           reject(request);
         };
 
-        request.open('POST', this.url);
+        request.open('POST', sentryReq.url);
         for (const header in this.options.headers) {
           if (this.options.headers.hasOwnProperty(header)) {
             request.setRequestHeader(header, this.options.headers[header]);
           }
         }
-        request.send(JSON.stringify(event));
+        request.send(sentryReq.body);
       }),
     );
   }
diff --git a/packages/browser/test/unit/transports/base.test.ts b/packages/browser/test/unit/transports/base.test.ts
@@ -19,6 +19,7 @@ describe('BaseTransport', () => {
 
   it('has correct endpoint url', () => {
     const transport = new SimpleTransport({ dsn: testDsn });
+    // tslint:disable-next-line:deprecation
     expect(transport.url).equal('https://sentry.io/api/42/store/?sentry_key=123&sentry_version=7');
   });
 });
diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts
@@ -28,6 +28,7 @@ describe('FetchTransport', () => {
   });
 
   it('inherits composeEndpointUrl() implementation', () => {
+    // tslint:disable-next-line:deprecation
     expect(transport.url).equal(transportUrl);
   });
 
diff --git a/packages/browser/test/unit/transports/xhr.test.ts b/packages/browser/test/unit/transports/xhr.test.ts
@@ -28,6 +28,7 @@ describe('XHRTransport', () => {
   });
 
   it('inherits composeEndpointUrl() implementation', () => {
+    // tslint:disable-next-line:deprecation
     expect(transport.url).equal(transportUrl);
   });
 
diff --git a/packages/core/src/api.ts b/packages/core/src/api.ts
@@ -17,29 +17,59 @@ export class API {
     return this._dsnObject;
   }
 
-  /** Returns a string with auth headers in the url to the store endpoint. */
+  /** Returns the prefix to construct Sentry ingestion API endpoints. */
+  public getBaseApiEndpoint(): string {
+    const dsn = this._dsnObject;
+    const protocol = dsn.protocol ? `${dsn.protocol}:` : '';
+    const port = dsn.port ? `:${dsn.port}` : '';
+    return `${protocol}//${dsn.host}${port}${dsn.path ? `/${dsn.path}` : ''}/api/`;
+  }
+
+  /** Returns the store endpoint URL. */
   public getStoreEndpoint(): string {
-    return `${this._getBaseUrl()}${this.getStoreEndpointPath()}`;
+    return this._getIngestEndpoint('store');
+  }
+
+  /** Returns the envelope endpoint URL. */
+  private _getEnvelopeEndpoint(): string {
+    return this._getIngestEndpoint('envelope');
+  }
+
+  /** Returns the ingest API endpoint for target. */
+  private _getIngestEndpoint(target: 'store' | 'envelope'): string {
+    const base = this.getBaseApiEndpoint();
+    const dsn = this._dsnObject;
+    return `${base}${dsn.projectId}/${target}/`;
   }
 
-  /** Returns the store endpoint with auth added in url encoded. */
+  /**
+   * Returns the store endpoint URL with auth in the query string.
+   *
+   * Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests.
+   */
   public getStoreEndpointWithUrlEncodedAuth(): string {
+    return `${this.getStoreEndpoint()}?${this._encodedAuth()}`;
+  }
+
+  /**
+   * Returns the envelope endpoint URL with auth in the query string.
+   *
+   * Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests.
+   */
+  public getEnvelopeEndpointWithUrlEncodedAuth(): string {
+    return `${this._getEnvelopeEndpoint()}?${this._encodedAuth()}`;
+  }
+
+  /** Returns a URL-encoded string with auth config suitable for a query string. */
+  private _encodedAuth(): string {
     const dsn = this._dsnObject;
     const auth = {
-      sentry_key: dsn.user, // sentry_key is currently used in tracing integration to identify internal sentry requests
+      // We send only the minimum set of required information. See
+      // https://github.com/getsentry/sentry-javascript/issues/2572.
+      sentry_key: dsn.user,
       sentry_version: SENTRY_API_VERSION,
     };
-    // Auth is intentionally sent as part of query string (NOT as custom HTTP header)
-    // to avoid preflight CORS requests
-    return `${this.getStoreEndpoint()}?${urlEncode(auth)}`;
-  }
-
-  /** Returns the base path of the url including the port. */
-  private _getBaseUrl(): string {
-    const dsn = this._dsnObject;
-    const protocol = dsn.protocol ? `${dsn.protocol}:` : '';
-    const port = dsn.port ? `:${dsn.port}` : '';
-    return `${protocol}//${dsn.host}${port}`;
+    return urlEncode(auth);
   }
 
   /** Returns only the path component for the store endpoint. */
@@ -48,7 +78,11 @@ export class API {
     return `${dsn.path ? `/${dsn.path}` : ''}/api/${dsn.projectId}/store/`;
   }
 
-  /** Returns an object that can be used in request headers. */
+  /**
+   * Returns an object that can be used in request headers.
+   *
+   * @deprecated in favor of `getStoreEndpointWithUrlEncodedAuth` and `getEnvelopeEndpointWithUrlEncodedAuth`.
+   */
   public getRequestHeaders(clientName: string, clientVersion: string): { [key: string]: string } {
     const dsn = this._dsnObject;
     const header = [`Sentry sentry_version=${SENTRY_API_VERSION}`];
@@ -71,7 +105,7 @@ export class API {
     } = {},
   ): string {
     const dsn = this._dsnObject;
-    const endpoint = `${this._getBaseUrl()}${dsn.path ? `/${dsn.path}` : ''}/api/embed/error-page/`;
+    const endpoint = `${this.getBaseApiEndpoint()}embed/error-page/`;
 
     const encodedOptions = [];
     encodedOptions.push(`dsn=${dsn.toString()}`);
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -16,6 +16,7 @@ export { addGlobalEventProcessor, getCurrentHub, getHubFromCarrier, Hub, Scope }
 export { API } from './api';
 export { BaseClient } from './baseclient';
 export { BackendClass, BaseBackend } from './basebackend';
+export { eventToSentryRequest } from './request';
 export { initAndBind, ClientClass } from './sdk';
 export { NoopTransport } from './transports/noop';
 
diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts
@@ -0,0 +1,61 @@
+import { Event } from '@sentry/types';
+
+import { API } from './api';
+
+/** A generic client request. */
+interface SentryRequest {
+  body: string;
+  url: string;
+  // headers would contain auth & content-type headers for @sentry/node, but
+  // since @sentry/browser avoids custom headers to prevent CORS preflight
+  // requests, we can use the same approach for @sentry/browser and @sentry/node
+  // for simplicity -- no headers involved.
+  // headers: { [key: string]: string };
+}
+
+/** Creates a SentryRequest from an event. */
+export function eventToSentryRequest(event: Event, api: API): SentryRequest {
+  const useEnvelope = event.type === 'transaction';
+
+  const req: SentryRequest = {
+    body: JSON.stringify(event),
+    url: useEnvelope ? api.getEnvelopeEndpointWithUrlEncodedAuth() : api.getStoreEndpointWithUrlEncodedAuth(),
+  };
+
+  // https://develop.sentry.dev/sdk/envelopes/
+
+  // Since we don't need to manipulate envelopes nor store them, there is no
+  // exported concept of an Envelope with operations including serialization and
+  // deserialization. Instead, we only implement a minimal subset of the spec to
+  // serialize events inline here.
+  if (useEnvelope) {
+    const envelopeHeaders = JSON.stringify({
+      event_id: event.event_id,
+      sent_at: new Date().toISOString(),
+    });
+    const itemHeaders = JSON.stringify({
+      type: event.type,
+      // The content-type is assumed to be 'application/json' and not part of
+      // the current spec for transaction items, so we don't bloat the request
+      // body with it.
+      //
+      // content_type: 'application/json',
+      //
+      // The length is optional. It must be the number of bytes in req.Body
+      // encoded as UTF-8. Since the server can figure this out and would
+      // otherwise refuse events that report the length incorrectly, we decided
+      // not to send the length to avoid problems related to reporting the wrong
+      // size and to reduce request body size.
+      //
+      // length: new TextEncoder().encode(req.body).length,
+    });
+    // The trailing newline is optional. We intentionally don't send it to avoid
+    // sending unnecessary bytes.
+    //
+    // const envelope = `${envelopeHeaders}\n${itemHeaders}\n${req.body}\n`;
+    const envelope = `${envelopeHeaders}\n${itemHeaders}\n${req.body}`;
+    req.body = envelope;
+  }
+
+  return req;
+}
diff --git a/packages/core/test/lib/api.test.ts b/packages/core/test/lib/api.test.ts
@@ -16,11 +16,13 @@ describe('API', () => {
   });
 
   test('getRequestHeaders', () => {
+    // tslint:disable-next-line:deprecation
     expect(new API(dsnPublic).getRequestHeaders('a', '1.0')).toMatchObject({
       'Content-Type': 'application/json',
       'X-Sentry-Auth': expect.stringMatching(/^Sentry sentry_version=\d, sentry_client=a\/1\.0, sentry_key=abc$/),
     });
 
+    // tslint:disable-next-line:deprecation
     expect(new API(legacyDsn).getRequestHeaders('a', '1.0')).toMatchObject({
       'Content-Type': 'application/json',
       'X-Sentry-Auth': expect.stringMatching(
diff --git a/packages/node/src/transports/base.ts b/packages/node/src/transports/base.ts
diff --git a/packages/node/test/transports/http.test.ts b/packages/node/test/transports/http.test.ts
diff --git a/packages/node/test/transports/https.test.ts b/packages/node/test/transports/https.test.ts
