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 <devoncarew@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>

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",

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<Null> 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.

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';
 }

pkg/analysis_server/lib/src/domain_server.dart

@@ -67,8 +67,6 @@ class ServerDomainHandler implements RequestHandler {
    * Cleanly shutdown the analysis server.
    */
   Future<Null> 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);

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");

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);
   }
 }
 

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) {

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}';
 

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<String, String> 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'));
 }

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);
   });
 }