From f1c1dfacb763549abdad4ac9dc38ca7eeb36f258 Mon Sep 17 00:00:00 2001 From: Mike Fairhurst Date: Mon, 4 May 2020 18:11:54 +0000 Subject: [PATCH] [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 --- .../lib/src/server/crash_reporting.dart | 40 +++++++++++-------- .../lib/src/server/driver.dart | 10 +++-- pkg/telemetry/lib/crash_reporting.dart | 35 ++++++++++++---- pkg/telemetry/test/crash_reporting_test.dart | 10 ++--- 4 files changed, 62 insertions(+), 33 deletions(-) diff --git a/pkg/analysis_server/lib/src/server/crash_reporting.dart b/pkg/analysis_server/lib/src/server/crash_reporting.dart index 879adbc8afe..8b5a118c448 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: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 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 91c6074b4fa..dc8af6286e9 100644 --- a/pkg/analysis_server/lib/src/server/driver.dart +++ b/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); diff --git a/pkg/telemetry/lib/crash_reporting.dart b/pkg/telemetry/lib/crash_reporting.dart index 1590da89160..7b4b958fd4b 100644 --- a/pkg/telemetry/lib/crash_reporting.dart +++ b/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. /// diff --git a/pkg/telemetry/test/crash_reporting_test.dart b/pkg/telemetry/test/crash_reporting_test.dart index be9342496a9..440eee96934 100644 --- a/pkg/telemetry/test/crash_reporting_test.dart +++ b/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);