[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>
diff --git a/pkg/analysis_server/lib/src/server/crash_reporting.dart b/pkg/analysis_server/lib/src/server/crash_reporting.dart index 879adbc..8b5a118 100644 --- a/pkg/analysis_server/lib/src/server/crash_reporting.dart +++ b/pkg/analysis_server/lib/src/server/crash_reporting.dart
@@ -9,9 +9,13 @@ 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 @@ // 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 @@ 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, ...). });
diff --git a/pkg/analysis_server/lib/src/server/driver.dart b/pkg/analysis_server/lib/src/server/driver.dart index 91c6074..dc8af62 100644 --- a/pkg/analysis_server/lib/src/server/driver.dart +++ b/pkg/analysis_server/lib/src/server/driver.dart
@@ -403,8 +403,10 @@ // 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 @@ } var errorNotifier = ErrorNotifier(); - allInstrumentationServices - .add(CrashReportingInstrumentation(crashReportSender)); + allInstrumentationServices.add(CrashReportingInstrumentation( + crashReportSenderStaging, crashReportSenderProd)); instrumentationService = MulticastInstrumentationService(allInstrumentationServices);
diff --git a/pkg/telemetry/lib/crash_reporting.dart b/pkg/telemetry/lib/crash_reporting.dart index 1590da8..7b4b958 100644 --- a/pkg/telemetry/lib/crash_reporting.dart +++ b/pkg/telemetry/lib/crash_reporting.dart
@@ -18,9 +18,11 @@ /// 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 @@ /// /// 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 @@ 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. ///
diff --git a/pkg/telemetry/test/crash_reporting_test.dart b/pkg/telemetry/test/crash_reporting_test.dart index be93424..440eee9 100644 --- a/pkg/telemetry/test/crash_reporting_test.dart +++ b/pkg/telemetry/test/crash_reporting_test.dart
@@ -31,7 +31,7 @@ }; test('general', () async { - CrashReportSender sender = new CrashReportSender( + CrashReportSender sender = new CrashReportSender.prod( analytics.trackingId, shouldSend, httpClient: mockClient); @@ -43,7 +43,7 @@ }); test('reportsSent', () async { - CrashReportSender sender = new CrashReportSender( + CrashReportSender sender = new CrashReportSender.prod( analytics.trackingId, shouldSend, httpClient: mockClient); @@ -59,7 +59,7 @@ }); test('contains message', () async { - CrashReportSender sender = new CrashReportSender( + CrashReportSender sender = new CrashReportSender.prod( analytics.trackingId, shouldSend, httpClient: mockClient); @@ -73,7 +73,7 @@ }); test('has attachments', () async { - CrashReportSender sender = new CrashReportSender( + CrashReportSender sender = new CrashReportSender.prod( analytics.trackingId, shouldSend, httpClient: mockClient); @@ -94,7 +94,7 @@ }); test('has ptime', () async { - CrashReportSender sender = new CrashReportSender( + CrashReportSender sender = new CrashReportSender.prod( analytics.trackingId, shouldSend, httpClient: mockClient);