Introduce event categories for every outcome

kamilogorek · kamilogorek · commit e5ce2b0648ad · 2021-09-07T15:01:54.000+02:00
diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts
@@ -35,7 +35,7 @@ export abstract class BaseTransport implements Transport {
   /** Locks transport after receiving rate limits in a response */
   protected readonly _rateLimits: Record<string, Date> = {};
 
-  protected _outcomes: { [key in Outcome]?: number } = {};
+  protected _outcomes: { [key: string]: number } = {};
 
   public constructor(public options: TransportOptions) {
     this._api = new API(options.dsn, options._metadata, options.tunnel);
@@ -68,13 +68,13 @@ export abstract class BaseTransport implements Transport {
   /**
    * @inheritDoc
    */
-  public recordLostEvent(type: Outcome): void {
+  public recordLostEvent(type: Outcome, category: SentryRequestType): void {
     if (!this.options.sendClientReport) {
       return;
     }
-
-    logger.log(`Adding ${type} outcome`);
-    this._outcomes[type] = (this._outcomes[type] ?? 0) + 1;
+    const key = `${type}:${CATEGORY_MAPPING[category]}`;
+    logger.log(`Adding ${key} outcome`);
+    this._outcomes[key] = (this._outcomes[key] ?? 0) + 1;
   }
 
   /**
@@ -106,7 +106,14 @@ export abstract class BaseTransport implements Transport {
     });
     const item = JSON.stringify({
       timestamp: Date.now(),
-      discarded_events: this._outcomes,
+      discarded_events: Object.keys(outcomes).map(key => {
+        const [category, reason] = key.split(':');
+        return {
+          reason,
+          category,
+          quantity: outcomes[key],
+        };
+      }),
     });
     const envelope = `${itemHeaders}\n${item}`;
 
diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts
@@ -113,7 +113,7 @@ export class FetchTransport extends BaseTransport {
    */
   private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
     if (this._isRateLimited(sentryRequest.type)) {
-      this.recordLostEvent(Outcome.RateLimit);
+      this.recordLostEvent(Outcome.RateLimit, sentryRequest.type);
 
       return Promise.reject({
         event: originalPayload,
@@ -164,9 +164,9 @@ export class FetchTransport extends BaseTransport {
       )
       .then(undefined, reason => {
         if (reason instanceof SentryError) {
-          this.recordLostEvent(Outcome.QueueSize);
+          this.recordLostEvent(Outcome.QueueSize, sentryRequest.type);
         } else {
-          this.recordLostEvent(Outcome.NetworkError);
+          this.recordLostEvent(Outcome.NetworkError, sentryRequest.type);
         }
         throw reason;
       });
diff --git a/packages/browser/src/transports/xhr.ts b/packages/browser/src/transports/xhr.ts
@@ -26,7 +26,7 @@ export class XHRTransport extends BaseTransport {
    */
   private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
     if (this._isRateLimited(sentryRequest.type)) {
-      this.recordLostEvent(Outcome.RateLimit);
+      this.recordLostEvent(Outcome.RateLimit, sentryRequest.type);
 
       return Promise.reject({
         event: originalPayload,
@@ -65,9 +65,9 @@ export class XHRTransport extends BaseTransport {
       )
       .then(undefined, reason => {
         if (reason instanceof SentryError) {
-          this.recordLostEvent(Outcome.QueueSize);
+          this.recordLostEvent(Outcome.QueueSize, sentryRequest.type);
         } else {
-          this.recordLostEvent(Outcome.NetworkError);
+          this.recordLostEvent(Outcome.NetworkError, sentryRequest.type);
         }
         throw reason;
       });
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -10,6 +10,7 @@ import {
   Outcome,
   SessionStatus,
   Severity,
+  Transport,
 } from '@sentry/types';
 import {
   dateTimestampInSeconds,
@@ -182,13 +183,19 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     return this._options;
   }
 
+  /**
+   * @inheritDoc
+   */
+  public getTransport(): Transport {
+    return this.getTransport();
+  }
+
   /**
    * @inheritDoc
    */
   public flush(timeout?: number): PromiseLike<boolean> {
     return this._isClientDoneProcessing(timeout).then(clientFinished => {
-      return this._getBackend()
-        .getTransport()
+      return this.getTransport()
         .close(timeout)
         .then(transportFlushed => clientFinished && transportFlushed);
     });
@@ -499,7 +506,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
   protected _processEvent(event: Event, hint?: EventHint, scope?: Scope): PromiseLike<Event> {
     // eslint-disable-next-line @typescript-eslint/unbound-method
     const { beforeSend, sampleRate } = this.getOptions();
-    const transport = this._getBackend().getTransport();
+    const transport = this.getTransport();
 
     if (!this._isEnabled()) {
       return SyncPromise.reject(new SentryError('SDK not enabled, will not capture event.'));
@@ -510,7 +517,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     // 0.0 === 0% events are sent
     // Sampling for transaction happens somewhere else
     if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) {
-      transport.recordLostEvent?.(Outcome.SampleRate);
+      transport.recordLostEvent?.(Outcome.SampleRate, 'event');
       return SyncPromise.reject(
         new SentryError(
           `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
@@ -521,7 +528,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     return this._prepareEvent(event, scope, hint)
       .then(prepared => {
         if (prepared === null) {
-          transport.recordLostEvent?.(Outcome.EventProcessor);
+          transport.recordLostEvent?.(Outcome.EventProcessor, event.type || 'event');
           throw new SentryError('An event processor returned null, will not send event.');
         }
 
@@ -535,7 +542,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
       })
       .then(processedEvent => {
         if (processedEvent === null) {
-          transport.recordLostEvent?.(Outcome.BeforeSend);
+          transport.recordLostEvent?.(Outcome.BeforeSend, event.type || 'event');
           throw new SentryError('`beforeSend` returned `null`, will not send event.');
         }
 
diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts
@@ -88,6 +88,16 @@ describe('BaseClient', () => {
     });
   });
 
+  describe('getTransport()', () => {
+    test('returns the transport from backend', () => {
+      expect.assertions(1);
+      const options = { dsn: PUBLIC_DSN, transport: FakeTransport };
+      const client = new TestClient(options);
+      expect(client.getTransport()).toBeInstanceOf(FakeTransport);
+      expect(TestBackend.instance!.getTransport()).toBe(client.getTransport());
+    });
+  });
+
   describe('getBreadcrumbs() / addBreadcrumb()', () => {
     test('adds a breadcrumb', () => {
       expect.assertions(1);
@@ -906,7 +916,7 @@ describe('BaseClient', () => {
       });
 
       const delay = 1;
-      const transportInstance = (client as any)._getBackend().getTransport() as FakeTransport;
+      const transportInstance = client.getTransport() as FakeTransport;
       transportInstance.delay = delay;
 
       client.captureMessage('test');
@@ -935,7 +945,7 @@ describe('BaseClient', () => {
             setTimeout(() => resolve({ message, level }), 150);
           }),
       );
-      const transportInstance = (client as any)._getBackend().getTransport() as FakeTransport;
+      const transportInstance = client.getTransport() as FakeTransport;
       transportInstance.delay = delay;
 
       client.captureMessage('test async');
@@ -959,7 +969,7 @@ describe('BaseClient', () => {
       });
 
       const delay = 1;
-      const transportInstance = (client as any)._getBackend().getTransport() as FakeTransport;
+      const transportInstance = client.getTransport() as FakeTransport;
       transportInstance.delay = delay;
 
       expect(client.captureMessage('test')).toBeTruthy();
diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts
@@ -98,7 +98,7 @@ export class NodeClient extends BaseClient<NodeBackend, NodeOptions> {
     if (!release) {
       logger.warn('Cannot initialise an instance of SessionFlusher if no release is provided!');
     } else {
-      this._sessionFlusher = new SessionFlusher(this._backend.getTransport(), {
+      this._sessionFlusher = new SessionFlusher(this.getTransport(), {
         release,
         environment,
       });
diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts
@@ -2,6 +2,7 @@ import { getCurrentHub, Hub } from '@sentry/hub';
 import {
   Event,
   Measurements,
+  Outcome,
   Transaction as TransactionInterface,
   TransactionContext,
   TransactionMetadata,
@@ -102,6 +103,12 @@ export class Transaction extends SpanClass implements TransactionInterface {
     if (this.sampled !== true) {
       // At this point if `sampled !== true` we want to discard the transaction.
       logger.log('[Tracing] Discarding transaction because its trace was not chosen to be sampled.');
+
+      this._hub
+        .getClient()
+        ?.getTransport()
+        .recordLostEvent?.(Outcome.SampleRate, 'transaction');
+
       return undefined;
     }
 
diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts
@@ -5,6 +5,7 @@ import { Options } from './options';
 import { Scope } from './scope';
 import { Session } from './session';
 import { Severity } from './severity';
+import { Transport } from './transport';
 
 /**
  * User-Facing Sentry SDK Client.
@@ -59,6 +60,9 @@ export interface Client<O extends Options = Options> {
   /** Returns the current options. */
   getOptions(): O;
 
+  /** Returns clients transport. */
+  getTransport(): Transport;
+
   /**
    * Flush the event queue and set the client to `enabled = false`. See {@link Client.flush}.
    *
diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts
@@ -1,5 +1,6 @@
 import { DsnLike } from './dsn';
 import { Event } from './event';
+import { SentryRequestType } from './request';
 import { Response } from './response';
 import { SdkMetadata } from './sdkmetadata';
 import { Session, SessionAggregates } from './session';
@@ -42,7 +43,7 @@ export interface Transport {
   /**
    * Increment the counter for the specific client outcome
    */
-  recordLostEvent?(type: Outcome): void;
+  recordLostEvent?(type: Outcome, category: SentryRequestType): void;
 }
 
 /** JSDoc */
