[analysis_server] Report to both prod & staging crash reporting

This CL enables us to have different crash reporters that report to
different backends, this will be useful if we report plugin errors to
staging and server errors to prod.

Currently doing this with server errors so that we can make the switch
over to prod incrementally. For the time being, the staging backend will
have our whole crash history, and continue to get new crashes. Reports
and dashboards can switch over to the prod backend as we get enough
history for those reports to be useful, and when we have switched
everything over we can stop reporting to staging entirely.

Change-Id: Ie29b4e05c89fd57faf1487a2c2eba3701fc319d0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/145922
Reviewed-by: Devon Carew <devoncarew@google.com>

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

@@ -9,9 +9,13 @@ import 'package:analyzer/instrumentation/service.dart';
 import 'package:telemetry/crash_reporting.dart';
 
 class CrashReportingInstrumentation extends NoopInstrumentationService {
-  final CrashReportSender reporter;
+  // A staging reporter, that we are in the process of phasing out.
+  final CrashReportSender stagingReporter;
 
-  CrashReportingInstrumentation(this.reporter);
+  // A prod reporter, that we are in the process of phasing in.
+  final CrashReportSender prodReporter;
+
+  CrashReportingInstrumentation(this.stagingReporter, this.prodReporter);
 
   @override
   void logException(dynamic exception,
@@ -28,19 +32,11 @@ class CrashReportingInstrumentation extends NoopInstrumentationService {
       // Get the root CaughtException, which matters most for debugging.
       var root = exception.rootCaughtException;
 
-      reporter
-          .sendReport(root.exception, root.stackTrace,
-              attachments: crashReportAttachments, comment: root.message)
-          .catchError((error) {
-        // We silently ignore errors sending crash reports (network issues, ...).
-      });
+      _sendServerReport(root.exception, root.stackTrace,
+          attachments: crashReportAttachments, comment: root.message);
     } else {
-      reporter
-          .sendReport(exception, stackTrace ?? StackTrace.current,
-              attachments: crashReportAttachments)
-          .catchError((error) {
-        // We silently ignore errors sending crash reports (network issues, ...).
-      });
+      _sendServerReport(exception, stackTrace ?? StackTrace.current,
+          attachments: crashReportAttachments);
     }
   }
 
@@ -57,8 +53,20 @@ class CrashReportingInstrumentation extends NoopInstrumentationService {
       return;
     }
 
-    reporter
-        .sendReport(exception, stackTrace, comment: 'plugin: ${plugin.name}')
+    _sendServerReport(exception, stackTrace, comment: 'plugin: ${plugin.name}');
+  }
+
+  void _sendServerReport(Object exception, Object stackTrace,
+      {String comment, List<CrashReportAttachment> attachments}) {
+    stagingReporter
+        .sendReport(exception, stackTrace,
+            attachments: attachments, comment: comment)
+        .catchError((error) {
+      // We silently ignore errors sending crash reports (network issues, ...).
+    });
+    prodReporter
+        .sendReport(exception, stackTrace,
+            attachments: attachments, comment: comment)
         .catchError((error) {
       // We silently ignore errors sending crash reports (network issues, ...).
     });

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

@@ -403,8 +403,10 @@ class Driver implements ServerStarter {
 
     // Use sdkConfig to optionally override analytics settings.
     final crashProductId = sdkConfig.crashReportingId ?? 'Dart_analysis_server';
-    final crashReportSender =
-        CrashReportSender(crashProductId, shouldSendCallback);
+    final crashReportSenderStaging =
+        CrashReportSender.staging(crashProductId, shouldSendCallback);
+    final crashReportSenderProd =
+        CrashReportSender.prod(crashProductId, shouldSendCallback);
 
     if (telemetry.SHOW_ANALYTICS_UI) {
       if (results.wasParsed(ANALYTICS_FLAG)) {
@@ -455,8 +457,8 @@ class Driver implements ServerStarter {
     }
 
     var errorNotifier = ErrorNotifier();
-    allInstrumentationServices
-        .add(CrashReportingInstrumentation(crashReportSender));
+    allInstrumentationServices.add(CrashReportingInstrumentation(
+        crashReportSenderStaging, crashReportSenderProd));
     instrumentationService =
         MulticastInstrumentationService(allInstrumentationServices);
 

pkg/telemetry/lib/crash_reporting.dart

@@ -18,9 +18,11 @@ const String _dartTypeId = 'DartError';
 /// Crash backend host.
 const String _crashServerHost = 'clients2.google.com';
 
-// This should be one of 'report' or 'staging_report'.
-/// Path to the crash servlet.
-const String _crashEndpointPath = '/cr/staging_report';
+/// Path to the staging crash servlet.
+const String _crashEndpointPathStaging = '/cr/staging_report';
+
+/// Path to the prod crash servlet.
+const String _crashEndpointPathProd = '/cr/report';
 
 /// The field corresponding to the multipart/form-data file attachment where
 /// crash backend expects to find the Dart stack trace.
@@ -36,8 +38,7 @@ const String _stackTraceFilename = 'stacktrace_file';
 ///
 /// Clients shouldn't extend, mixin or implement this class.
 class CrashReportSender {
-  static final Uri _baseUri = new Uri(
-      scheme: 'https', host: _crashServerHost, path: _crashEndpointPath);
+  final Uri _baseUri;
 
   static const int _maxReportsToSend = 1000;
 
@@ -50,12 +51,30 @@ class CrashReportSender {
   int _reportsSent = 0;
   int _skippedReports = 0;
 
-  /// Create a new [CrashReportSender].
-  CrashReportSender(
+  CrashReportSender._(
     this.crashProductId,
     this.shouldSend, {
     http.Client httpClient,
-  }) : _httpClient = httpClient ?? new http.Client();
+    String endpointPath = _crashEndpointPathStaging,
+  })  : _httpClient = httpClient ?? new http.Client(),
+        _baseUri = new Uri(
+            scheme: 'https', host: _crashServerHost, path: endpointPath);
+
+  /// Create a new [CrashReportSender] connected to the staging endpoint.
+  CrashReportSender.staging(
+    String crashProductId,
+    EnablementCallback shouldSend, {
+    http.Client httpClient,
+  }) : this._(crashProductId, shouldSend,
+            httpClient: httpClient, endpointPath: _crashEndpointPathStaging);
+
+  /// Create a new [CrashReportSender] connected to the prod endpoint.
+  CrashReportSender.prod(
+    String crashProductId,
+    EnablementCallback shouldSend, {
+    http.Client httpClient,
+  }) : this._(crashProductId, shouldSend,
+            httpClient: httpClient, endpointPath: _crashEndpointPathProd);
 
   /// Sends one crash report.
   ///

pkg/telemetry/test/crash_reporting_test.dart

@@ -31,7 +31,7 @@ void main() {
     };
 
     test('general', () async {
-      CrashReportSender sender = new CrashReportSender(
+      CrashReportSender sender = new CrashReportSender.prod(
           analytics.trackingId, shouldSend,
           httpClient: mockClient);
 
@@ -43,7 +43,7 @@ void main() {
     });
 
     test('reportsSent', () async {
-      CrashReportSender sender = new CrashReportSender(
+      CrashReportSender sender = new CrashReportSender.prod(
           analytics.trackingId, shouldSend,
           httpClient: mockClient);
 
@@ -59,7 +59,7 @@ void main() {
     });
 
     test('contains message', () async {
-      CrashReportSender sender = new CrashReportSender(
+      CrashReportSender sender = new CrashReportSender.prod(
           analytics.trackingId, shouldSend,
           httpClient: mockClient);
 
@@ -73,7 +73,7 @@ void main() {
     });
 
     test('has attachments', () async {
-      CrashReportSender sender = new CrashReportSender(
+      CrashReportSender sender = new CrashReportSender.prod(
           analytics.trackingId, shouldSend,
           httpClient: mockClient);
 
@@ -94,7 +94,7 @@ void main() {
     });
 
     test('has ptime', () async {
-      CrashReportSender sender = new CrashReportSender(
+      CrashReportSender sender = new CrashReportSender.prod(
           analytics.trackingId, shouldSend,
           httpClient: mockClient);