[analyzer] send additional data to crash reporting

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

pkg/analysis_server/lib/src/analysis_server.dart

@@ -356,7 +356,7 @@ class AnalysisServer extends AbstractAnalysisServer {
       });
     }, onError: (exception, stackTrace) {
       AnalysisEngine.instance.instrumentationService.logException(
-          FatalException('Failed to handle request: ${request.toJson()}',
+          FatalException('Failed to handle request: ${request.method}',
               exception, stackTrace));
     });
   }

pkg/analysis_server/lib/src/domain_completion.dart

@@ -258,7 +258,7 @@ class CompletionDomainHandler extends AbstractRequestHandler {
     }, onError: (exception, stackTrace) {
       AnalysisEngine.instance.instrumentationService.logException(
           CaughtException.withMessage(
-              'Failed to handle completion domain request: ${request.toJson()}',
+              'Failed to handle completion domain request: ${request.method}',
               exception,
               stackTrace));
     });

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

@@ -18,7 +18,9 @@ class CrashReportingInstrumentation extends NoopInstrumentationService {
       // Get the root CaughtException, which matters most for debugging.
       CaughtException root = exception.rootCaughtException;
 
-      reporter.sendReport(root.exception, root.stackTrace).catchError((error) {
+      reporter
+          .sendReport(root.exception, root.stackTrace, comment: root.message)
+          .catchError((error) {
         // We silently ignore errors sending crash reports (network issues, ...).
       });
     } else {

pkg/telemetry/lib/crash_reporting.dart

@@ -4,8 +4,10 @@
 
 import 'dart:async';
 import 'dart:io';
+import 'dart:math' as math;
 
 import 'package:http/http.dart' as http;
+import 'package:meta/meta.dart';
 import 'package:stack_trace/stack_trace.dart';
 
 import 'src/utils.dart';
@@ -42,9 +44,11 @@ class CrashReportSender {
   final String crashProductId;
   final EnablementCallback shouldSend;
   final http.Client _httpClient;
+  final Stopwatch _processStopwatch = new Stopwatch()..start();
 
   final ThrottlingBucket _throttle = ThrottlingBucket(10, Duration(minutes: 1));
-  int _reportsSend = 0;
+  int _reportsSent = 0;
+  int _skippedReports = 0;
 
   /// Create a new [CrashReportSender].
   CrashReportSender(
@@ -56,21 +60,36 @@ class CrashReportSender {
   /// Sends one crash report.
   ///
   /// The report is populated from data in [error] and [stackTrace].
-  Future sendReport(dynamic error, StackTrace stackTrace) async {
+  ///
+  /// Additional context about the crash can optionally be passed in via
+  /// [comment]. Note that this field should not include PII.
+  Future sendReport(
+    dynamic error,
+    StackTrace stackTrace, {
+    String comment,
+  }) async {
     if (!shouldSend()) {
       return;
     }
 
     // Check if we've sent too many reports recently.
     if (!_throttle.removeDrop()) {
+      _skippedReports++;
       return;
     }
 
     // Don't send too many total reports to crash reporting.
-    if (_reportsSend >= _maxReportsToSend) {
+    if (_reportsSent >= _maxReportsToSend) {
       return;
     }
 
+    _reportsSent++;
+
+    // Calculate the 'weight' of the this report; we increase the weight of a
+    // report if we had throttled previous reports.
+    int weight = math.min(_skippedReports + 1, 10000);
+    _skippedReports = 0;
+
     try {
       final String dartVersion = Platform.version.split(' ').first;
 
@@ -82,13 +101,28 @@ class CrashReportSender {
       );
 
       final http.MultipartRequest req = new http.MultipartRequest('POST', uri);
-      req.fields['product'] = crashProductId;
-      req.fields['version'] = dartVersion;
-      req.fields['osName'] = Platform.operatingSystem;
-      req.fields['osVersion'] = Platform.operatingSystemVersion;
-      req.fields['type'] = _dartTypeId;
-      req.fields['error_runtime_type'] = '${error.runtimeType}';
-      req.fields['error_message'] = '$error';
+
+      Map<String, String> fields = req.fields;
+      fields['product'] = crashProductId;
+      fields['version'] = dartVersion;
+      fields['osName'] = Platform.operatingSystem;
+      fields['osVersion'] = Platform.operatingSystemVersion;
+      fields['type'] = _dartTypeId;
+      fields['error_runtime_type'] = '${error.runtimeType}';
+      fields['error_message'] = '$error';
+
+      // Optional comments.
+      if (comment != null) {
+        fields['comments'] = comment;
+      }
+
+      // The uptime of the process before it crashed (in milliseconds).
+      fields['ptime'] = _processStopwatch.elapsedMilliseconds.toString();
+
+      // Send the amount to weight this report.
+      if (weight > 1) {
+        fields['weight'] = weight.toString();
+      }
 
       final Chain chain = new Chain.forTrace(stackTrace);
       req.files.add(new http.MultipartFile.fromString(
@@ -108,6 +142,9 @@ class CrashReportSender {
     }
   }
 
+  @visibleForTesting
+  int get reportsSent => _reportsSent;
+
   /// Closes the client and cleans up any resources associated with it. This
   /// will close the associated [http.Client].
   void dispose() {

pkg/telemetry/test/crash_reporting_test.dart

@@ -11,8 +11,9 @@ import 'package:test/test.dart';
 import 'package:usage/usage.dart';
 
 void main() {
-  group('crash_reporting', () {
+  group('CrashReportSender', () {
     MockClient mockClient;
+    AnalyticsMock analytics;
 
     Request request;
 
@@ -21,14 +22,15 @@ void main() {
         request = r;
         return new Response('crash-report-001', 200);
       });
+
+      analytics = new AnalyticsMock()..enabled = true;
     });
 
-    test('CrashReportSender', () async {
-      EnablementCallback shouldSend = () {
-        return true;
-      };
+    EnablementCallback shouldSend = () {
+      return true;
+    };
 
-      AnalyticsMock analytics = new AnalyticsMock()..enabled = true;
+    test('general', () async {
       CrashReportSender sender = new CrashReportSender(
           analytics.trackingId, shouldSend,
           httpClient: mockClient);
@@ -39,5 +41,46 @@ void main() {
       expect(body, contains('String')); // error.runtimeType
       expect(body, contains('test-error'));
     });
+
+    test('reportsSent', () async {
+      CrashReportSender sender = new CrashReportSender(
+          analytics.trackingId, shouldSend,
+          httpClient: mockClient);
+
+      expect(sender.reportsSent, 0);
+
+      await sender.sendReport('test-error', StackTrace.current);
+
+      expect(sender.reportsSent, 1);
+
+      String body = utf8.decode(request.bodyBytes);
+      expect(body, contains('String'));
+      expect(body, contains('test-error'));
+    });
+
+    test('contains message', () async {
+      CrashReportSender sender = new CrashReportSender(
+          analytics.trackingId, shouldSend,
+          httpClient: mockClient);
+
+      await sender.sendReport('test-error', StackTrace.current,
+          comment: 'additional message');
+
+      String body = utf8.decode(request.bodyBytes);
+      expect(body, contains('String'));
+      expect(body, contains('test-error'));
+      expect(body, contains('additional message'));
+    });
+
+    test('has ptime', () async {
+      CrashReportSender sender = new CrashReportSender(
+          analytics.trackingId, shouldSend,
+          httpClient: mockClient);
+
+      await sender.sendReport('test-error', StackTrace.current);
+
+      String body = utf8.decode(request.bodyBytes);
+      expect(body, contains('name="ptime"'));
+    });
   });
 }