Unverified Commit cf0d4979 authored by Zachary Anderson's avatar Zachary Anderson Committed by GitHub

[flutter_tool] Re-work analytics events to use labels and values (#42016)

parent 2d642e95
...@@ -27,7 +27,13 @@ class DisabledUsage implements Usage { ...@@ -27,7 +27,13 @@ class DisabledUsage implements Usage {
void sendCommand(String command, { Map<String, String> parameters }) { } void sendCommand(String command, { Map<String, String> parameters }) { }
@override @override
void sendEvent(String category, String parameter, { String label, Map<String, String> parameters }) { } void sendEvent(
String category,
String parameter, {
String label,
int value,
Map<String, String> parameters,
}) { }
@override @override
void sendTiming(String category, String variableName, Duration duration, { String label }) { } void sendTiming(String category, String variableName, Duration duration, { String label }) { }
......
...@@ -11,14 +11,16 @@ part of reporting; ...@@ -11,14 +11,16 @@ part of reporting;
class UsageEvent { class UsageEvent {
UsageEvent(this.category, this.parameter, { UsageEvent(this.category, this.parameter, {
this.label, this.label,
this.value,
}); });
final String category; final String category;
final String parameter; final String parameter;
final String label; final String label;
final int value;
void send() { void send() {
flutterUsage.sendEvent(category, parameter, label: label); flutterUsage.sendEvent(category, parameter, label: label, value: value);
} }
} }
...@@ -95,8 +97,11 @@ class DoctorResultEvent extends UsageEvent { ...@@ -95,8 +97,11 @@ class DoctorResultEvent extends UsageEvent {
DoctorResultEvent({ DoctorResultEvent({
@required this.validator, @required this.validator,
@required this.result, @required this.result,
}) : super('doctorResult.${validator.runtimeType}', }) : super(
result.typeStr); 'doctor-result',
'${validator.runtimeType}',
label: result.typeStr,
);
final DoctorValidator validator; final DoctorValidator validator;
final ValidationResult result; final ValidationResult result;
...@@ -116,24 +121,29 @@ class DoctorResultEvent extends UsageEvent { ...@@ -116,24 +121,29 @@ class DoctorResultEvent extends UsageEvent {
} }
} }
/// An event that reports success or failure of a pub get. /// An event that reports on the result of a pub invocation.
class PubResultEvent extends UsageEvent { class PubResultEvent extends UsageEvent {
PubResultEvent({ PubResultEvent({
@required String context, @required String context,
@required String result, @required String result,
}) : super('pub', context, label: result); }) : super('pub-result', context, label: result);
} }
/// An event that reports something about a build. /// An event that reports something about a build.
class BuildEvent extends UsageEvent { class BuildEvent extends UsageEvent {
BuildEvent(String parameter, { BuildEvent(String label, {
this.command, this.command,
this.settings, this.settings,
this.eventError, this.eventError,
}) : super( }) : super(
'build' + // category
(FlutterCommand.current == null ? '' : '-${FlutterCommand.current.name}'), 'build',
parameter); // parameter
FlutterCommand.current == null ?
'unspecified' :
'${FlutterCommand.current.name}',
label: label,
);
final String command; final String command;
final String settings; final String settings;
...@@ -149,7 +159,12 @@ class BuildEvent extends UsageEvent { ...@@ -149,7 +159,12 @@ class BuildEvent extends UsageEvent {
if (eventError != null) if (eventError != null)
CustomDimensions.buildEventError: eventError, CustomDimensions.buildEventError: eventError,
}); });
flutterUsage.sendEvent(category, parameter, parameters: parameters); flutterUsage.sendEvent(
category,
parameter,
label: label,
parameters: parameters,
);
} }
} }
...@@ -160,17 +175,27 @@ class CommandResultEvent extends UsageEvent { ...@@ -160,17 +175,27 @@ class CommandResultEvent extends UsageEvent {
@override @override
void send() { void send() {
int maxRss; // An event for the command result.
flutterUsage.sendEvent(
'tool-command-result',
category,
label: parameter,
);
// A separate event for the memory highwater mark. This is a separate event
// so that we can get the command result even if trying to grab maxRss
// throws an exception.
try { try {
maxRss = processInfo.maxRss; final int maxRss = processInfo.maxRss;
} catch (e) { flutterUsage.sendEvent(
// If grabbing the maxRss fails for some reason, just leave it off the 'tool-command-max-rss',
// event. category,
label: parameter,
value: maxRss,
);
} catch (error) {
// If grabbing the maxRss fails for some reason, just don't send an event.
printTrace('Querying maxRss failed with error: $error');
} }
final Map<String, String> parameters = _useCdKeys(<CustomDimensions, String>{
if (maxRss != null)
CustomDimensions.commandResultEventMaxRss: maxRss.toString(),
});
flutterUsage.sendEvent(category, parameter, parameters: parameters);
} }
} }
...@@ -125,6 +125,7 @@ abstract class Usage { ...@@ -125,6 +125,7 @@ abstract class Usage {
String category, String category,
String parameter, { String parameter, {
String label, String label,
int value,
Map<String, String> parameters, Map<String, String> parameters,
}); });
...@@ -264,6 +265,7 @@ class _DefaultUsage implements Usage { ...@@ -264,6 +265,7 @@ class _DefaultUsage implements Usage {
String category, String category,
String parameter, { String parameter, {
String label, String label,
int value,
Map<String, String> parameters, Map<String, String> parameters,
}) { }) {
if (suppressAnalytics) { if (suppressAnalytics) {
...@@ -279,6 +281,7 @@ class _DefaultUsage implements Usage { ...@@ -279,6 +281,7 @@ class _DefaultUsage implements Usage {
category, category,
parameter, parameter,
label: label, label: label,
value: value,
parameters: paramsWithLocalTime, parameters: paramsWithLocalTime,
); );
} }
......
...@@ -226,9 +226,9 @@ void main() { ...@@ -226,9 +226,9 @@ void main() {
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PassingValidator', 'doctor-result',
captureAny, 'PassingValidator',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['installed', 'installed', 'installed'], <dynamic>['installed', 'installed', 'installed'],
); );
...@@ -243,25 +243,25 @@ void main() { ...@@ -243,25 +243,25 @@ void main() {
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PassingValidator', 'doctor-result',
captureAny, 'PassingValidator',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['installed', 'installed'], <dynamic>['installed', 'installed'],
); );
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PartialValidatorWithHintsOnly', 'doctor-result',
captureAny, 'PartialValidatorWithHintsOnly',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['partial'], <dynamic>['partial'],
); );
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PartialValidatorWithErrors', 'doctor-result',
captureAny, 'PartialValidatorWithErrors',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['partial'], <dynamic>['partial'],
); );
...@@ -275,41 +275,41 @@ void main() { ...@@ -275,41 +275,41 @@ void main() {
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PassingValidator', 'doctor-result',
captureAny, 'PassingValidator',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['installed'], <dynamic>['installed'],
); );
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.MissingValidator', 'doctor-result',
captureAny, 'MissingValidator',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['missing'], <dynamic>['missing'],
); );
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.NotAvailableValidator', 'doctor-result',
captureAny, 'NotAvailableValidator',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['notAvailable'], <dynamic>['notAvailable'],
); );
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PartialValidatorWithHintsOnly', 'doctor-result',
captureAny, 'PartialValidatorWithHintsOnly',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['partial'], <dynamic>['partial'],
); );
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PartialValidatorWithErrors', 'doctor-result',
captureAny, 'PartialValidatorWithErrors',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['partial'], <dynamic>['partial'],
); );
...@@ -323,17 +323,17 @@ void main() { ...@@ -323,17 +323,17 @@ void main() {
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.PassingGroupedValidator', 'doctor-result',
captureAny, 'PassingGroupedValidator',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['installed', 'installed', 'installed'], <dynamic>['installed', 'installed', 'installed'],
); );
expect( expect(
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'doctorResult.MissingGroupedValidator', 'doctor-result',
captureAny, 'MissingGroupedValidator',
label: anyNamed('label'), label: captureAnyNamed('label'),
)).captured, )).captured,
<dynamic>['missing'], <dynamic>['missing'],
); );
......
...@@ -55,7 +55,7 @@ void main() { ...@@ -55,7 +55,7 @@ void main() {
flutterUsage.enabled = true; flutterUsage.enabled = true;
await createProject(tempDir); await createProject(tempDir);
expect(count, flutterUsage.isFirstRun ? 0 : 3); expect(count, flutterUsage.isFirstRun ? 0 : 4);
count = 0; count = 0;
flutterUsage.enabled = false; flutterUsage.enabled = false;
......
...@@ -298,8 +298,9 @@ flutter: ...@@ -298,8 +298,9 @@ flutter:
contains('To learn more, see: https://developer.android.com/studio/build/shrink-code')); contains('To learn more, see: https://developer.android.com/studio/build/shrink-code'));
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'build-apk', 'build',
'r8-failure', 'apk',
label: 'r8-failure',
parameters: anyNamed('parameters'), parameters: anyNamed('parameters'),
)).called(1); )).called(1);
}, },
......
...@@ -289,8 +289,9 @@ flutter: ...@@ -289,8 +289,9 @@ flutter:
contains('To learn more, see: https://developer.android.com/studio/build/shrink-code')); contains('To learn more, see: https://developer.android.com/studio/build/shrink-code'));
verify(mockUsage.sendEvent( verify(mockUsage.sendEvent(
'build-appbundle', 'build',
'r8-failure', 'appbundle',
label: 'r8-failure',
parameters: anyNamed('parameters'), parameters: anyNamed('parameters'),
)).called(1); )).called(1);
}, },
......
...@@ -154,7 +154,7 @@ void main() { ...@@ -154,7 +154,7 @@ void main() {
testUsingContext('analytics sent on success', () async { testUsingContext('analytics sent on success', () async {
MockDirectory.findCache = true; MockDirectory.findCache = true;
await pubGet(context: PubContext.flutterTests, checkLastModified: false); await pubGet(context: PubContext.flutterTests, checkLastModified: false);
verify(flutterUsage.sendEvent('pub', 'flutter-tests', label: 'success')).called(1); verify(flutterUsage.sendEvent('pub-result', 'flutter-tests', label: 'success')).called(1);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
ProcessManager: () => MockProcessManager(0), ProcessManager: () => MockProcessManager(0),
FileSystem: () => MockFileSystem(), FileSystem: () => MockFileSystem(),
...@@ -173,7 +173,7 @@ void main() { ...@@ -173,7 +173,7 @@ void main() {
} on ToolExit { } on ToolExit {
// Ignore. // Ignore.
} }
verify(flutterUsage.sendEvent('pub', 'flutter-tests', label: 'failure')).called(1); verify(flutterUsage.sendEvent('pub-result', 'flutter-tests', label: 'failure')).called(1);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
ProcessManager: () => MockProcessManager(1), ProcessManager: () => MockProcessManager(1),
FileSystem: () => MockFileSystem(), FileSystem: () => MockFileSystem(),
...@@ -192,7 +192,7 @@ void main() { ...@@ -192,7 +192,7 @@ void main() {
} on ToolExit { } on ToolExit {
// Ignore. // Ignore.
} }
verify(flutterUsage.sendEvent('pub', 'flutter-tests', label: 'version-solving-failed')).called(1); verify(flutterUsage.sendEvent('pub-result', 'flutter-tests', label: 'version-solving-failed')).called(1);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
ProcessManager: () => MockProcessManager( ProcessManager: () => MockProcessManager(
1, 1,
......
...@@ -246,7 +246,9 @@ void main() { ...@@ -246,7 +246,9 @@ void main() {
); );
await diagnoseXcodeBuildFailure(buildResult); await diagnoseXcodeBuildFailure(buildResult);
verify(mockUsage.sendEvent('build', 'xcode-bitcode-failure', verify(mockUsage.sendEvent('build',
any,
label: 'xcode-bitcode-failure',
parameters: <String, String>{ parameters: <String, String>{
cdKey(CustomDimensions.buildEventCommand): buildCommands.toString(), cdKey(CustomDimensions.buildEventCommand): buildCommands.toString(),
cdKey(CustomDimensions.buildEventSettings): buildSettings.toString(), cdKey(CustomDimensions.buildEventSettings): buildSettings.toString(),
......
...@@ -77,8 +77,24 @@ void main() { ...@@ -77,8 +77,24 @@ void main() {
); );
await flutterCommand.run(); await flutterCommand.run();
verify(usage.sendCommand(captureAny, parameters: captureAnyNamed('parameters'))); verify(usage.sendCommand(
verify(usage.sendEvent(captureAny, 'success', parameters: captureAnyNamed('parameters'))); 'dummy',
parameters: anyNamed('parameters'),
));
verify(usage.sendEvent(
'tool-command-result',
'dummy',
label: 'success',
parameters: anyNamed('parameters'),
));
expect(verify(usage.sendEvent(
'tool-command-max-rss',
'dummy',
label: 'success',
value: captureAnyNamed('value'),
)).captured[0],
10,
);
}); });
testUsingCommandContext('reports command that results in warning', () async { testUsingCommandContext('reports command that results in warning', () async {
...@@ -92,8 +108,24 @@ void main() { ...@@ -92,8 +108,24 @@ void main() {
); );
await flutterCommand.run(); await flutterCommand.run();
verify(usage.sendCommand(captureAny, parameters: captureAnyNamed('parameters'))); verify(usage.sendCommand(
verify(usage.sendEvent(captureAny, 'warning', parameters: captureAnyNamed('parameters'))); 'dummy',
parameters: anyNamed('parameters'),
));
verify(usage.sendEvent(
'tool-command-result',
'dummy',
label: 'warning',
parameters: anyNamed('parameters'),
));
expect(verify(usage.sendEvent(
'tool-command-max-rss',
'dummy',
label: 'warning',
value: captureAnyNamed('value'),
)).captured[0],
10,
);
}); });
testUsingCommandContext('reports command that results in failure', () async { testUsingCommandContext('reports command that results in failure', () async {
...@@ -109,8 +141,24 @@ void main() { ...@@ -109,8 +141,24 @@ void main() {
try { try {
await flutterCommand.run(); await flutterCommand.run();
} on ToolExit { } on ToolExit {
verify(usage.sendCommand(captureAny, parameters: captureAnyNamed('parameters'))); verify(usage.sendCommand(
verify(usage.sendEvent(captureAny, 'fail', parameters: captureAnyNamed('parameters'))); 'dummy',
parameters: anyNamed('parameters'),
));
verify(usage.sendEvent(
'tool-command-result',
'dummy',
label: 'fail',
parameters: anyNamed('parameters'),
));
expect(verify(usage.sendEvent(
'tool-command-max-rss',
'dummy',
label: 'fail',
value: captureAnyNamed('value'),
)).captured[0],
10,
);
} }
}); });
...@@ -129,8 +177,24 @@ void main() { ...@@ -129,8 +177,24 @@ void main() {
await flutterCommand.run(); await flutterCommand.run();
fail('Mock should make this fail'); fail('Mock should make this fail');
} on ToolExit { } on ToolExit {
verify(usage.sendCommand(captureAny, parameters: captureAnyNamed('parameters'))); verify(usage.sendCommand(
verify(usage.sendEvent(captureAny, 'fail', parameters: captureAnyNamed('parameters'))); 'dummy',
parameters: anyNamed('parameters'),
));
verify(usage.sendEvent(
'tool-command-result',
'dummy',
label: 'fail',
parameters: anyNamed('parameters'),
));
expect(verify(usage.sendEvent(
'tool-command-max-rss',
'dummy',
label: 'fail',
value: captureAnyNamed('value'),
)).captured[0],
10,
);
} }
}); });
...@@ -169,8 +233,24 @@ void main() { ...@@ -169,8 +233,24 @@ void main() {
signalController.add(mockSignal); signalController.add(mockSignal);
await completer.future; await completer.future;
verify(usage.sendCommand(any, parameters: anyNamed('parameters'))); verify(usage.sendCommand(
verify(usage.sendEvent(any, 'killed', parameters: anyNamed('parameters'))); 'dummy',
parameters: anyNamed('parameters'),
));
verify(usage.sendEvent(
'tool-command-result',
'dummy',
label: 'killed',
parameters: anyNamed('parameters'),
));
expect(verify(usage.sendEvent(
'tool-command-max-rss',
'dummy',
label: 'killed',
value: captureAnyNamed('value'),
)).captured[0],
10,
);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
ProcessInfo: () => mockProcessInfo, ProcessInfo: () => mockProcessInfo,
Signals: () => FakeSignals( Signals: () => FakeSignals(
...@@ -182,27 +262,6 @@ void main() { ...@@ -182,27 +262,6 @@ void main() {
}); });
}); });
testUsingCommandContext('reports maxRss', () async {
// Crash if called a third time which is unexpected.
mockTimes = <int>[1000, 2000];
final DummyFlutterCommand flutterCommand = DummyFlutterCommand(
commandFunction: () async {
return const FlutterCommandResult(ExitStatus.success);
}
);
await flutterCommand.run();
verify(usage.sendCommand(captureAny, parameters: captureAnyNamed('parameters')));
expect(verify(usage.sendEvent(
any,
'success',
parameters: captureAnyNamed('parameters'),
)).captured[0],
containsPair(cdKey(CustomDimensions.commandResultEventMaxRss),
mockProcessInfo.maxRss.toString()));
});
testUsingCommandContext('report execution timing by default', () async { testUsingCommandContext('report execution timing by default', () async {
// Crash if called a third time which is unexpected. // Crash if called a third time which is unexpected.
mockTimes = <int>[1000, 2000]; mockTimes = <int>[1000, 2000];
......
...@@ -140,8 +140,15 @@ class CrashingUsage implements Usage { ...@@ -140,8 +140,15 @@ class CrashingUsage implements Usage {
String category, String category,
String parameter, { String parameter, {
String label, String label,
int value,
Map<String, String> parameters, Map<String, String> parameters,
}) => _impl.sendEvent(category, parameter, label: label, parameters: parameters); }) => _impl.sendEvent(
category,
parameter,
label: label,
value: value,
parameters: parameters,
);
@override @override
void sendTiming( void sendTiming(
......
...@@ -303,7 +303,11 @@ class FakeUsage implements Usage { ...@@ -303,7 +303,11 @@ class FakeUsage implements Usage {
void sendCommand(String command, { Map<String, String> parameters }) { } void sendCommand(String command, { Map<String, String> parameters }) { }
@override @override
void sendEvent(String category, String parameter, { String label, Map<String, String> parameters }) { } void sendEvent(String category, String parameter, {
String label,
int value,
Map<String, String> parameters,
}) { }
@override @override
void sendTiming(String category, String variableName, Duration duration, { String label }) { } void sendTiming(String category, String variableName, Duration duration, { String label }) { }
......
...@@ -162,7 +162,11 @@ class NoOpUsage implements Usage { ...@@ -162,7 +162,11 @@ class NoOpUsage implements Usage {
void sendCommand(String command, {Map<String, String> parameters}) {} void sendCommand(String command, {Map<String, String> parameters}) {}
@override @override
void sendEvent(String category, String parameter, { String label, Map<String, String> parameters }) {} void sendEvent(String category, String parameter, {
String label,
int value,
Map<String, String> parameters,
}) {}
@override @override
void sendException(dynamic exception) {} void sendException(dynamic exception) {}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment