From c19233dcce7eaf111790e7937cff1ea6f3a6085f Mon Sep 17 00:00:00 2001 From: Devon Carew Date: Thu, 28 Jun 2018 16:28:25 +0000 Subject: [PATCH] Plumb in more of the analytics impl into the analysis server; allow clients to enble analytics. Change-Id: Id546ce60e3ec49d37d6c5545f98a46bff009e6c1 Reviewed-on: https://dart-review.googlesource.com/62701 Commit-Queue: Devon Carew Reviewed-by: Brian Wilkerson --- DEPS | 2 +- .../lib/src/analysis_server.dart | 24 ++++--- .../lib/src/domain_analytics.dart | 32 ++++++--- .../lib/src/domain_server.dart | 2 - .../lib/src/server/driver.dart | 15 ++-- .../integration/analytics/enable_test.dart | 11 +-- pkg/analyzer_cli/lib/src/options.dart | 7 +- pkg/telemetry/lib/crash_reporting.dart | 8 ++- pkg/telemetry/lib/telemetry.dart | 70 ++++++++++++------- pkg/telemetry/test/crash_reporting_test.dart | 2 +- 10 files changed, 107 insertions(+), 66 deletions(-) diff --git a/DEPS b/DEPS index 0bff841741e..8bea544225e 100644 --- a/DEPS +++ b/DEPS @@ -135,7 +135,7 @@ vars = { "test_tag": "1.0.0", "tuple_tag": "v1.0.1", "typed_data_tag": "1.1.3", - "usage_tag": "3.3.0", + "usage_tag": "3.4.0", "utf_tag": "0.9.0+4", "watcher_tag": "0.9.7+8", "web_components_rev": "8f57dac273412a7172c8ade6f361b407e2e4ed02", diff --git a/pkg/analysis_server/lib/src/analysis_server.dart b/pkg/analysis_server/lib/src/analysis_server.dart index 6fb9452a492..bd45c00f062 100644 --- a/pkg/analysis_server/lib/src/analysis_server.dart +++ b/pkg/analysis_server/lib/src/analysis_server.dart @@ -339,6 +339,11 @@ class AnalysisServer { */ DiagnosticServer diagnosticServer; + /** + * The analytics instance; note, this object can be `null`. + */ + telemetry.Analytics get analytics => options.analytics; + /** * Initialize a newly created server to receive requests from and send * responses to the given [channel]. @@ -853,12 +858,13 @@ class AnalysisServer { // send to crash reporting if (options.crashReportSender != null) { - // Catch and ignore any exceptions when reporting exceptions (network - // errors or other). options.crashReportSender .sendReport(exception, stackTrace: stackTrace is StackTrace ? stackTrace : null) - .catchError((_) {}); + .catchError((_) { + // Catch and ignore any exceptions when reporting exceptions (network + // errors or other). + }); } // remember the last few exceptions @@ -1010,13 +1016,15 @@ class AnalysisServer { } Future shutdown() async { - // TODO(brianwilkerson) Determine whether this await is necessary. - await null; running = false; - await options.analytics - ?.waitForLastPing(timeout: new Duration(milliseconds: 200)); - options.analytics?.close(); + if (options.analytics != null) { + options.analytics + .waitForLastPing(timeout: new Duration(milliseconds: 200)) + .then((_) { + options.analytics.close(); + }); + } // Defer closing the channel and shutting down the instrumentation server so // that the shutdown response can be sent and logged. diff --git a/pkg/analysis_server/lib/src/domain_analytics.dart b/pkg/analysis_server/lib/src/domain_analytics.dart index f6ad37e675a..220fe27e7eb 100644 --- a/pkg/analysis_server/lib/src/domain_analytics.dart +++ b/pkg/analysis_server/lib/src/domain_analytics.dart @@ -8,33 +8,35 @@ import 'package:analysis_server/protocol/protocol.dart'; import 'package:analysis_server/protocol/protocol_constants.dart'; import 'package:analysis_server/protocol/protocol_generated.dart'; import 'package:analysis_server/src/analysis_server.dart'; +import 'package:telemetry/telemetry.dart'; /// Instances of the class [AnalyticsDomainHandler] implement a [RequestHandler] /// that handles requests in the `analytics` domain. class AnalyticsDomainHandler implements RequestHandler { final AnalysisServer server; - bool enabled = false; + Analytics get analytics => server.analytics; AnalyticsDomainHandler(this.server); - // TODO(devoncarew): This implementation is currently mocked out. Response handleEnable(Request request) { - // TODO(devoncarew): Implement. AnalyticsEnableParams params = new AnalyticsEnableParams.fromRequest(request); - enabled = params.value; + if (analytics != null) { + analytics.enabled = params.value; + } return new AnalyticsEnableResult().toResponse(request.id); } Response handleIsEnabled(Request request) { - // TODO(devoncarew): Implement. - return new AnalyticsIsEnabledResult(enabled).toResponse(request.id); + return new AnalyticsIsEnabledResult(analytics?.enabled ?? false) + .toResponse(request.id); } @override Response handleRequest(Request request) { String requestName = request.method; + if (requestName == ANALYTICS_REQUEST_IS_ENABLED) { return handleIsEnabled(request); } else if (requestName == ANALYTICS_REQUEST_ENABLE) { @@ -49,12 +51,26 @@ class AnalyticsDomainHandler implements RequestHandler { } Response handleSendEvent(Request request) { - // TODO(devoncarew): Implement. + if (analytics == null) { + return new AnalyticsSendEventResult().toResponse(request.id); + } + + AnalyticsSendEventParams params = + new AnalyticsSendEventParams.fromRequest(request); + analytics.sendEvent(_clientId, params.action); return new AnalyticsSendEventResult().toResponse(request.id); } Response handleSendTiming(Request request) { - // TODO(devoncarew): Implement. + if (analytics == null) { + return new AnalyticsSendTimingResult().toResponse(request.id); + } + + AnalyticsSendTimingParams params = + new AnalyticsSendTimingParams.fromRequest(request); + analytics.sendTiming(params.event, params.millis, category: _clientId); return new AnalyticsSendTimingResult().toResponse(request.id); } + + String get _clientId => server.options.clientId ?? 'client'; } diff --git a/pkg/analysis_server/lib/src/domain_server.dart b/pkg/analysis_server/lib/src/domain_server.dart index 1f205bbe4b5..ccd12bc9dbc 100644 --- a/pkg/analysis_server/lib/src/domain_server.dart +++ b/pkg/analysis_server/lib/src/domain_server.dart @@ -67,8 +67,6 @@ class ServerDomainHandler implements RequestHandler { * Cleanly shutdown the analysis server. */ Future shutdown(Request request) async { - // TODO(brianwilkerson) Determine whether this await is necessary. - await null; await server.shutdown(); Response response = new ServerShutdownResult().toResponse(request.id); server.sendResponse(response); diff --git a/pkg/analysis_server/lib/src/server/driver.dart b/pkg/analysis_server/lib/src/server/driver.dart index 612290a4e3a..1d11ede9e4b 100644 --- a/pkg/analysis_server/lib/src/server/driver.dart +++ b/pkg/analysis_server/lib/src/server/driver.dart @@ -382,8 +382,8 @@ class Driver implements ServerStarter { } else { // No path to the SDK was provided. // Use FolderBasedDartSdk.defaultSdkDirectory, which will make a guess. - defaultSdkPath = FolderBasedDartSdk - .defaultSdkDirectory(PhysicalResourceProvider.INSTANCE) + defaultSdkPath = FolderBasedDartSdk.defaultSdkDirectory( + PhysicalResourceProvider.INSTANCE) .path; } // TODO(brianwilkerson) It would be nice to avoid creating an SDK that @@ -513,12 +513,13 @@ class Driver implements ServerStarter { negatable: false); parser.addOption(NEW_ANALYSIS_DRIVER_LOG, help: "set a destination for the new analysis driver's log"); - if (telemetry.SHOW_ANALYTICS_UI) { - parser.addFlag(ANALYTICS_FLAG, - help: 'enable or disable sending analytics information to Google'); - } + parser.addFlag(ANALYTICS_FLAG, + help: 'enable or disable sending analytics information to Google', + hide: !telemetry.SHOW_ANALYTICS_UI); parser.addFlag(SUPPRESS_ANALYTICS_FLAG, - negatable: false, help: 'suppress analytics for this session'); + negatable: false, + help: 'suppress analytics for this session', + hide: !telemetry.SHOW_ANALYTICS_UI); parser.addOption(PORT_OPTION, help: "the http diagnostic port on which the server provides" " status and performance information"); diff --git a/pkg/analysis_server/test/integration/analytics/enable_test.dart b/pkg/analysis_server/test/integration/analytics/enable_test.dart index d04e9eea893..0c1dbc0a574 100644 --- a/pkg/analysis_server/test/integration/analytics/enable_test.dart +++ b/pkg/analysis_server/test/integration/analytics/enable_test.dart @@ -20,16 +20,17 @@ class EnableTest extends AbstractAnalysisServerIntegrationTest { test_call_enable() async { standardAnalysisSetup(); - // Toggle the value twice, and verify the changes. + // Toggle the value twice; do light verification of the changes, as the + // analysis server - when running on our CI bots - deliberately does not + // send analytics info. AnalyticsIsEnabledResult result1 = await sendAnalyticsIsEnabled(); - await sendAnalyticsEnable(!result1.enabled); + expect(result1.enabled, isNotNull); + await sendAnalyticsEnable(!result1.enabled); AnalyticsIsEnabledResult result2 = await sendAnalyticsIsEnabled(); - expect(result2.enabled, !result1.enabled); + expect(result2.enabled, isNotNull); await sendAnalyticsEnable(result1.enabled); - result2 = await sendAnalyticsIsEnabled(); - expect(result2.enabled, result1.enabled); } } diff --git a/pkg/analyzer_cli/lib/src/options.dart b/pkg/analyzer_cli/lib/src/options.dart index 80450e839fd..d0521d30005 100644 --- a/pkg/analyzer_cli/lib/src/options.dart +++ b/pkg/analyzer_cli/lib/src/options.dart @@ -363,10 +363,9 @@ class CommandLineOptions { help: 'Verbose output.', negatable: false); - if (telemetry.SHOW_ANALYTICS_UI) { - parser.addFlag('analytics', - help: 'Enable or disable sending analytics information to Google.'); - } + parser.addFlag('analytics', + help: 'Enable or disable sending analytics information to Google.', + hide: !telemetry.SHOW_ANALYTICS_UI); // Build mode options. if (!hide) { diff --git a/pkg/telemetry/lib/crash_reporting.dart b/pkg/telemetry/lib/crash_reporting.dart index c34766eb0b8..ef43e3a5df1 100644 --- a/pkg/telemetry/lib/crash_reporting.dart +++ b/pkg/telemetry/lib/crash_reporting.dart @@ -9,6 +9,9 @@ import 'package:http/http.dart' as http; import 'package:stack_trace/stack_trace.dart'; import 'package:usage/usage.dart'; +// Reporting disabled until we're set up on the backend. +const bool CRASH_REPORTING_DISABLED = true; + /// Crash backend host. const String _crashServerHost = 'clients2.google.com'; @@ -46,7 +49,7 @@ class CrashReportSender { /// /// The report is populated from data in [error] and [stackTrace]. Future sendReport(dynamic error, {StackTrace stackTrace}) async { - if (!analytics.enabled) { + if (!analytics.enabled || CRASH_REPORTING_DISABLED) { return; } @@ -63,8 +66,7 @@ class CrashReportSender { req.fields['product'] = crashProductId; req.fields['version'] = analytics.applicationVersion; req.fields['osName'] = Platform.operatingSystem; - // TODO(devoncarew): Report the operating system version when we're able. - //req.fields['osVersion'] = Platform.operatingSystemVersion; + req.fields['osVersion'] = Platform.operatingSystemVersion; req.fields['type'] = 'DartError'; req.fields['error_runtime_type'] = '${error.runtimeType}'; diff --git a/pkg/telemetry/lib/telemetry.dart b/pkg/telemetry/lib/telemetry.dart index 7b05ec13cbe..6340cd2adc2 100644 --- a/pkg/telemetry/lib/telemetry.dart +++ b/pkg/telemetry/lib/telemetry.dart @@ -13,9 +13,6 @@ import 'package:usage/usage_io.dart'; export 'package:usage/usage.dart' show Analytics; -// TODO(devoncarew): Hard-coded to off for now. Remove when we're ready to ship. -final bool _HARD_CODE_OFF = true; - // TODO(devoncarew): Don't show the UI until we're ready to ship. final bool SHOW_ANALYTICS_UI = false; @@ -38,8 +35,10 @@ final String analyticsNotice = /// /// An example return value might be `'Analytics are currently enabled (and can /// be disabled with --no-analytics).'` -String createAnalyticsStatusMessage(bool enabled, - {String command: 'analytics'}) { +String createAnalyticsStatusMessage( + bool enabled, { + String command: 'analytics', +}) { String currentState = enabled ? 'enabled' : 'disabled'; String toggleState = enabled ? 'disabled' : 'enabled'; String commandToggle = enabled ? 'no-$command' : command; @@ -53,17 +52,16 @@ String createAnalyticsStatusMessage(bool enabled, /// /// This analytics instance will share a common enablement state with the rest /// of the Dart SDK tools. -Analytics createAnalyticsInstance(String trackingId, String applicationName, - {bool disableForSession: false}) { +_TelemetryAnalytics createAnalyticsInstance( + String trackingId, + String applicationName, { + bool disableForSession: false, +}) { Directory dir = getDartStorageDirectory(); if (!dir.existsSync()) { dir.createSync(); } - if (_HARD_CODE_OFF) { - disableForSession = true; - } - File file = new File(path.join(dir.path, _settingsFileName)); return new _TelemetryAnalytics( trackingId, applicationName, getDartVersion(), file, disableForSession); @@ -73,8 +71,9 @@ Analytics createAnalyticsInstance(String trackingId, String applicationName, /// /// Typically, the directory is `~/.dart/` (and the settings file is /// `analytics.json`). -Directory getDartStorageDirectory() => - new Directory(path.join(userHomeDir(), _dartDirectoryName)); +Directory getDartStorageDirectory() { + return new Directory(path.join(userHomeDir(), _dartDirectoryName)); +} /// Return the version of the Dart SDK. String getDartVersion() => usage_io.getDartVersion(); @@ -88,8 +87,7 @@ class _TelemetryAnalytics extends AnalyticsImpl { String applicationVersion, File file, this.disableForSession, - ) - : super( + ) : super( trackingId, new IOPersistentProperties.fromFile(file), new IOPostHandler(), @@ -107,20 +105,38 @@ class _TelemetryAnalytics extends AnalyticsImpl { if (disableForSession || isRunningOnBot()) { return false; } - return super.enabled; + + // If there's no explicit setting (enabled or disabled) then we don't send. + return (properties['enabled'] as bool) ?? false; } } bool isRunningOnBot() { - // - https://docs.travis-ci.com/user/environment-variables/ - // - https://www.appveyor.com/docs/environment-variables/ - // - CHROME_HEADLESS and BUILDBOT_BUILDERNAME are properties on Chrome infra - // bots. - return Platform.environment['TRAVIS'] == 'true' || - Platform.environment['BOT'] == 'true' || - Platform.environment['CONTINUOUS_INTEGRATION'] == 'true' || - Platform.environment['CHROME_HEADLESS'] == '1' || - Platform.environment.containsKey('BUILDBOT_BUILDERNAME') || - Platform.environment.containsKey('APPVEYOR') || - Platform.environment.containsKey('CI'); + final Map env = Platform.environment; + + return env['BOT'] != 'false' && + (env['BOT'] == 'true' + // https://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables + || + env['TRAVIS'] == 'true' || + env['CONTINUOUS_INTEGRATION'] == 'true' || + env.containsKey('CI') // Travis and AppVeyor + + // https://www.appveyor.com/docs/environment-variables/ + || + env.containsKey('APPVEYOR') + + // https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-env-vars.html + || + (env.containsKey('AWS_REGION') && + env.containsKey('CODEBUILD_INITIATOR')) + + // https://wiki.jenkins.io/display/JENKINS/Building+a+software+project#Buildingasoftwareproject-belowJenkinsSetEnvironmentVariables + || + env.containsKey('JENKINS_URL') + + // Properties on Flutter's Chrome Infra bots. + || + env['CHROME_HEADLESS'] == '1' || + env.containsKey('BUILDBOT_BUILDERNAME')); } diff --git a/pkg/telemetry/test/crash_reporting_test.dart b/pkg/telemetry/test/crash_reporting_test.dart index a61e1b49189..466e31e4194 100644 --- a/pkg/telemetry/test/crash_reporting_test.dart +++ b/pkg/telemetry/test/crash_reporting_test.dart @@ -36,6 +36,6 @@ void main() { expect(body, contains(analytics.trackingId)); expect(body, contains('1.0.0')); expect(body, contains(analytics.clientId)); - }); + }, skip: CRASH_REPORTING_DISABLED); }); }