Unverified Commit 7a278ae4 authored by Elias Yishak's avatar Elias Yishak Committed by GitHub

`CommandResultEvent` migrated (#138165)

Related to tracker issue:
- https://github.com/flutter/flutter/issues/128251

This event was only called from one file (`flutter_command.dart`). With the previous implementation, we actually sent 2 events, one for the result of the `commandPath` and another containing the `maxRss` value from `ProcessInfo`.

I have consolidated this down to just one event and used a function to safely get the `maxRss` value, or return null when if there was an error getting that integer value
parent af246a51
...@@ -189,8 +189,14 @@ class BuildEvent extends UsageEvent { ...@@ -189,8 +189,14 @@ class BuildEvent extends UsageEvent {
/// An event that reports the result of a top-level command. /// An event that reports the result of a top-level command.
class CommandResultEvent extends UsageEvent { class CommandResultEvent extends UsageEvent {
CommandResultEvent(super.commandPath, super.result) CommandResultEvent(
: super(flutterUsage: globals.flutterUsage); super.commandPath,
super.result,
int? maxRss,
) : _maxRss = maxRss,
super(flutterUsage: globals.flutterUsage);
final int? _maxRss;
@override @override
void send() { void send() {
...@@ -204,17 +210,13 @@ class CommandResultEvent extends UsageEvent { ...@@ -204,17 +210,13 @@ class CommandResultEvent extends UsageEvent {
// A separate event for the memory highwater mark. This is a separate event // 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 // so that we can get the command result even if trying to grab maxRss
// throws an exception. // throws an exception.
try { if (_maxRss != null) {
final int maxRss = globals.processInfo.maxRss;
flutterUsage.sendEvent( flutterUsage.sendEvent(
'tool-command-max-rss', 'tool-command-max-rss',
category, category,
label: parameter, label: parameter,
value: maxRss, value: _maxRss,
); );
} on Exception catch (error) {
// If grabbing the maxRss fails for some reason, just don't send an event.
globals.printTrace('Querying maxRss failed with error: $error');
} }
} }
} }
......
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
import 'package:unified_analytics/unified_analytics.dart'; import 'package:unified_analytics/unified_analytics.dart';
import '../base/io.dart';
import '../globals.dart' as globals;
import '../version.dart'; import '../version.dart';
/// This function is called from within the context runner to perform /// This function is called from within the context runner to perform
...@@ -52,3 +54,13 @@ Analytics getAnalytics({ ...@@ -52,3 +54,13 @@ Analytics getAnalytics({
clientIde: clientIde, clientIde: clientIde,
); );
} }
/// Function to safely grab the max rss from [ProcessInfo].
int? getMaxRss(ProcessInfo processInfo) {
try {
return globals.processInfo.maxRss;
} on Exception catch (error) {
globals.printTrace('Querying maxRss failed with error: $error');
}
return null;
}
...@@ -13,6 +13,7 @@ import '../application_package.dart'; ...@@ -13,6 +13,7 @@ import '../application_package.dart';
import '../base/common.dart'; import '../base/common.dart';
import '../base/context.dart'; import '../base/context.dart';
import '../base/io.dart' as io; import '../base/io.dart' as io;
import '../base/io.dart';
import '../base/os.dart'; import '../base/os.dart';
import '../base/user_messages.dart'; import '../base/user_messages.dart';
import '../base/utils.dart'; import '../base/utils.dart';
...@@ -30,6 +31,7 @@ import '../globals.dart' as globals; ...@@ -30,6 +31,7 @@ import '../globals.dart' as globals;
import '../preview_device.dart'; import '../preview_device.dart';
import '../project.dart'; import '../project.dart';
import '../reporting/reporting.dart'; import '../reporting/reporting.dart';
import '../reporting/unified_analytics.dart';
import '../web/compile.dart'; import '../web/compile.dart';
import 'flutter_command_runner.dart'; import 'flutter_command_runner.dart';
import 'target_devices.dart'; import 'target_devices.dart';
...@@ -214,6 +216,8 @@ abstract class FlutterCommand extends Command<void> { ...@@ -214,6 +216,8 @@ abstract class FlutterCommand extends Command<void> {
bool get deprecated => false; bool get deprecated => false;
ProcessInfo get processInfo => globals.processInfo;
/// When the command runs and this is true, trigger an async process to /// When the command runs and this is true, trigger an async process to
/// discover devices from discoverers that support wireless devices for an /// discover devices from discoverers that support wireless devices for an
/// extended amount of time and refresh the device cache with the results. /// extended amount of time and refresh the device cache with the results.
...@@ -1376,7 +1380,12 @@ abstract class FlutterCommand extends Command<void> { ...@@ -1376,7 +1380,12 @@ abstract class FlutterCommand extends Command<void> {
final DateTime endTime = globals.systemClock.now(); final DateTime endTime = globals.systemClock.now();
globals.printTrace(userMessages.flutterElapsedTime(name, getElapsedAsMilliseconds(endTime.difference(startTime)))); globals.printTrace(userMessages.flutterElapsedTime(name, getElapsedAsMilliseconds(endTime.difference(startTime))));
if (commandPath != null) { if (commandPath != null) {
_sendPostUsage(commandPath, commandResult, startTime, endTime); _sendPostUsage(
commandPath,
commandResult,
startTime,
endTime,
);
} }
if (_usesFatalWarnings) { if (_usesFatalWarnings) {
globals.logger.checkForFatalLogs(); globals.logger.checkForFatalLogs();
...@@ -1596,7 +1605,13 @@ abstract class FlutterCommand extends Command<void> { ...@@ -1596,7 +1605,13 @@ abstract class FlutterCommand extends Command<void> {
DateTime endTime, DateTime endTime,
) { ) {
// Send command result. // Send command result.
CommandResultEvent(commandPath, commandResult.toString()).send(); final int? maxRss = getMaxRss(processInfo);
CommandResultEvent(commandPath, commandResult.toString(), maxRss).send();
analytics.send(Event.flutterCommandResult(
commandPath: commandPath,
result: commandResult.toString(),
maxRss: maxRss,
));
// Send timing. // Send timing.
final List<String?> labels = <String?>[ final List<String?> labels = <String?>[
......
...@@ -27,10 +27,12 @@ import 'package:flutter_tools/src/project.dart'; ...@@ -27,10 +27,12 @@ import 'package:flutter_tools/src/project.dart';
import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:flutter_tools/src/reporting/reporting.dart';
import 'package:flutter_tools/src/runner/flutter_command.dart'; import 'package:flutter_tools/src/runner/flutter_command.dart';
import 'package:test/fake.dart'; import 'package:test/fake.dart';
import 'package:unified_analytics/unified_analytics.dart';
import '../../src/common.dart'; import '../../src/common.dart';
import '../../src/context.dart'; import '../../src/context.dart';
import '../../src/fake_devices.dart'; import '../../src/fake_devices.dart';
import '../../src/fakes.dart';
import '../../src/test_flutter_command_runner.dart'; import '../../src/test_flutter_command_runner.dart';
import 'utils.dart'; import 'utils.dart';
...@@ -38,6 +40,7 @@ void main() { ...@@ -38,6 +40,7 @@ void main() {
group('Flutter Command', () { group('Flutter Command', () {
late FakeCache cache; late FakeCache cache;
late TestUsage usage; late TestUsage usage;
late FakeAnalytics fakeAnalytics;
late FakeClock clock; late FakeClock clock;
late FakeProcessInfo processInfo; late FakeProcessInfo processInfo;
late MemoryFileSystem fileSystem; late MemoryFileSystem fileSystem;
...@@ -64,6 +67,10 @@ void main() { ...@@ -64,6 +67,10 @@ void main() {
logger = BufferLogger.test(); logger = BufferLogger.test();
processManager = FakeProcessManager.empty(); processManager = FakeProcessManager.empty();
preRunValidator = PreRunValidator(fileSystem: fileSystem); preRunValidator = PreRunValidator(fileSystem: fileSystem);
fakeAnalytics = getInitializedFakeAnalyticsInstance(
fs: fileSystem,
fakeFlutterVersion: FakeFlutterVersion(),
);
}); });
tearDown(() { tearDown(() {
...@@ -194,6 +201,7 @@ void main() { ...@@ -194,6 +201,7 @@ void main() {
ProcessManager: () => processManager, ProcessManager: () => processManager,
SystemClock: () => clock, SystemClock: () => clock,
Usage: () => usage, Usage: () => usage,
Analytics: () => fakeAnalytics,
}); });
} }
...@@ -221,6 +229,13 @@ void main() { ...@@ -221,6 +229,13 @@ void main() {
value: 10, value: 10,
), ),
]); ]);
expect(fakeAnalytics.sentEvents, <Event>[
Event.flutterCommandResult(
commandPath: 'dummy',
result: 'success',
maxRss: 10
),
]);
}); });
testUsingCommandContext('reports command that results in warning', () async { testUsingCommandContext('reports command that results in warning', () async {
...@@ -247,6 +262,13 @@ void main() { ...@@ -247,6 +262,13 @@ void main() {
value: 10, value: 10,
), ),
]); ]);
expect(fakeAnalytics.sentEvents, <Event>[
Event.flutterCommandResult(
commandPath: 'dummy',
result: 'warning',
maxRss: 10
),
]);
}); });
testUsingCommandContext('reports command that results in error', () async { testUsingCommandContext('reports command that results in error', () async {
...@@ -275,6 +297,13 @@ void main() { ...@@ -275,6 +297,13 @@ void main() {
value: 10, value: 10,
), ),
]); ]);
expect(fakeAnalytics.sentEvents, <Event>[
Event.flutterCommandResult(
commandPath: 'dummy',
result: 'fail',
maxRss: 10
),
]);
}); });
test('FlutterCommandResult.success()', () async { test('FlutterCommandResult.success()', () async {
...@@ -381,6 +410,13 @@ void main() { ...@@ -381,6 +410,13 @@ void main() {
value: 10, value: 10,
), ),
]); ]);
expect(fakeAnalytics.sentEvents, <Event>[
Event.flutterCommandResult(
commandPath: 'dummy',
result: 'killed',
maxRss: 10
),
]);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FileSystem: () => fileSystem, FileSystem: () => fileSystem,
ProcessManager: () => processManager, ProcessManager: () => processManager,
...@@ -391,6 +427,7 @@ void main() { ...@@ -391,6 +427,7 @@ void main() {
), ),
SystemClock: () => clock, SystemClock: () => clock,
Usage: () => usage, Usage: () => usage,
Analytics: () => fakeAnalytics,
}); });
testUsingContext('command release lock on kill signal', () async { testUsingContext('command release lock on kill signal', () async {
......
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