[analyzer] rate limit the crash reports we send

Change-Id: I9f6af7fbcec653a2a9396d1098bea678777d4b3b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124761
Reviewed-by: Mike Fairhurst <mfairhurst@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>

Author: devoncarew

pkg/analysis_server/lib/src/server/crash_reporting.dart

@@ -13,14 +13,19 @@ class CrashReportingInstrumentation extends NoopInstrumentationService {
 
   @override
   void logException(dynamic exception, [StackTrace stackTrace]) {
-    reporter.sendReport(exception,
-        stackTrace: stackTrace ?? StackTrace.current);
+    reporter
+        .sendReport(exception, stackTrace: stackTrace ?? StackTrace.current)
+        .catchError((error) {
+      // We silently ignore errors sending crash reports (network issues, ...).
+    });
   }
 
   @override
   void logPluginException(
       PluginData plugin, dynamic exception, StackTrace stackTrace) {
     // TODO(mfairhurst): send plugin information too.
-    reporter.sendReport(exception, stackTrace: stackTrace);
+    reporter.sendReport(exception, stackTrace: stackTrace).catchError((error) {
+      // We silently ignore errors sending crash reports (network issues, ...).
+    });
   }
 }

pkg/telemetry/lib/crash_reporting.dart

@@ -8,8 +8,7 @@ import 'dart:io';
 import 'package:http/http.dart' as http;
 import 'package:stack_trace/stack_trace.dart';
 
-// Reporting disabled until we're set up on the backend.
-const bool CRASH_REPORTING_DISABLED = true;
+import 'src/utils.dart';
 
 /// Tells crash backend that this is a Dart error (as opposed to, say, Java).
 const String _dartTypeId = 'DartError';
@@ -37,21 +36,37 @@ class CrashReportSender {
   static final Uri _baseUri = new Uri(
       scheme: 'https', host: _crashServerHost, path: _crashEndpointPath);
 
+  static const int _maxReportsToSend = 1000;
+
   final String crashProductId;
   final EnablementCallback shouldSend;
   final http.Client _httpClient;
 
-  /// Create a new [CrashReportSender], using the data from the given
-  /// [Analytics] instance.
-  CrashReportSender(this.crashProductId, this.shouldSend,
-      {http.Client httpClient})
-      : _httpClient = httpClient ?? new http.Client();
+  final ThrottlingBucket _throttle = ThrottlingBucket(10, Duration(minutes: 1));
+  int _reportsSend = 0;
+
+  /// Create a new [CrashReportSender].
+  CrashReportSender(
+    this.crashProductId,
+    this.shouldSend, {
+    http.Client httpClient,
+  }) : _httpClient = httpClient ?? new http.Client();
 
   /// Sends one crash report.
   ///
   /// The report is populated from data in [error] and [stackTrace].
   Future sendReport(dynamic error, {StackTrace stackTrace}) async {
-    if (!shouldSend() || CRASH_REPORTING_DISABLED) {
+    if (!shouldSend()) {
+      return;
+    }
+
+    // Check if we've sent too many reports recently.
+    if (!_throttle.removeDrop()) {
+      return;
+    }
+
+    // Don't send too many total reports to crash reporting.
+    if (_reportsSend >= _maxReportsToSend) {
       return;
     }
 

pkg/telemetry/lib/src/utils.dart

@@ -0,0 +1,47 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:math' as math;
+
+/// A throttling algorithm. This models the throttling after a bucket with
+/// water dripping into it at the rate of 1 drop per replenish duration. If the
+/// bucket has water when an operation is requested, 1 drop of water is removed
+/// and the operation is performed. If not the operation is skipped. This
+/// algorithm lets operations be performed in bursts without throttling, but
+/// holds the overall average rate of operations to 1 per replenish duration.
+class ThrottlingBucket {
+  final int bucketSize;
+  final Duration replenishDuration;
+
+  int _drops;
+  int _lastReplenish;
+
+  ThrottlingBucket(this.bucketSize, this.replenishDuration) {
+    _drops = bucketSize;
+    _lastReplenish = new DateTime.now().millisecondsSinceEpoch;
+  }
+
+  bool removeDrop() {
+    _checkReplenish();
+
+    if (_drops <= 0) {
+      return false;
+    } else {
+      _drops--;
+      return true;
+    }
+  }
+
+  void _checkReplenish() {
+    int now = new DateTime.now().millisecondsSinceEpoch;
+
+    int replenishMillis = replenishDuration.inMilliseconds;
+
+    if (_lastReplenish + replenishMillis >= now) {
+      int inc = (now - _lastReplenish) ~/ replenishMillis;
+      _drops = math.min(_drops + inc, bucketSize);
+      _lastReplenish += (replenishMillis * inc);
+    }
+  }
+}

pkg/telemetry/test/crash_reporting_test.dart

@@ -38,6 +38,6 @@ void main() {
       String body = utf8.decode(request.bodyBytes);
       expect(body, contains('String')); // error.runtimeType
       expect(body, contains('test-error'));
-    }, skip: CRASH_REPORTING_DISABLED);
+    });
   });
 }

pkg/telemetry/test/utils_test.dart

@@ -0,0 +1,23 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:telemetry/src/utils.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('ThrottlingBucket', () {
+    test('can send', () {
+      ThrottlingBucket bucket = new ThrottlingBucket(10, Duration(minutes: 1));
+      expect(bucket.removeDrop(), true);
+    });
+
+    test("doesn't send too many", () {
+      ThrottlingBucket bucket = new ThrottlingBucket(10, Duration(minutes: 1));
+      for (int i = 0; i < 10; i++) {
+        expect(bucket.removeDrop(), true);
+      }
+      expect(bucket.removeDrop(), false);
+    });
+  });
+}