Skip to content

Commit

Permalink
Send enabled_features as an event parameter rather than a user prop…
Browse files Browse the repository at this point in the history
…erty
  • Loading branch information
andrewkolos committed Jan 28, 2025
1 parent 8b73e68 commit 3934675
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 16 deletions.
7 changes: 6 additions & 1 deletion pkgs/unified_analytics/lib/src/analytics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ class AnalyticsImpl implements Analytics {
final File _clientIdFile;
final UserProperty _userProperty;
final LogHandler _logHandler;
final String? _enabledFeatures;

/// Tells the client if they need to show a message to the
/// user; this will return true if it is the first time the
Expand Down Expand Up @@ -406,8 +407,8 @@ class AnalyticsImpl implements Analytics {
truncateStringToLength(io.Platform.operatingSystemVersion, 36),
locale: io.Platform.localeName,
clientIde: clientIde,
enabledFeatures: enabledFeatures,
),
_enabledFeatures = enabledFeatures,
_configHandler = ConfigHandler(
homeDirectory: homeDirectory,
configFile: homeDirectory
Expand Down Expand Up @@ -613,6 +614,7 @@ class AnalyticsImpl implements Analytics {
eventName: event.eventName,
eventData: event.eventData,
userProperty: _userProperty,
enabledFeatures: _enabledFeatures,
);

if (_enableAsserts) checkBody(body);
Expand Down Expand Up @@ -654,6 +656,7 @@ class AnalyticsImpl implements Analytics {
eventName: collectionEvent.eventName,
eventData: collectionEvent.eventData,
userProperty: _userProperty,
enabledFeatures: _enabledFeatures,
);

_logHandler.save(data: body);
Expand All @@ -664,6 +667,7 @@ class AnalyticsImpl implements Analytics {
clientId: clientId,
eventName: collectionEvent.eventName,
eventData: collectionEvent.eventData,
enabledFeatures: _enabledFeatures,
userProperty: _userProperty,
);

Expand Down Expand Up @@ -774,6 +778,7 @@ class FakeAnalytics extends AnalyticsImpl {
eventName: event.eventName,
eventData: event.eventData,
userProperty: _userProperty,
enabledFeatures: _enabledFeatures,
);

if (_enableAsserts) checkBody(body);
Expand Down
1 change: 0 additions & 1 deletion pkgs/unified_analytics/lib/src/event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,6 @@ final class Event {
if (reloadVMTimeInMs != null) 'reloadVMTimeInMs': reloadVMTimeInMs,
};

// TODO: eliasyishak, add better dartdocs to explain each param
/// Event that is emitted periodically to report the number of times each lint
/// has been enabled.
///
Expand Down
3 changes: 0 additions & 3 deletions pkgs/unified_analytics/lib/src/user_property.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class UserProperty {
final String hostOsVersion;
final String locale;
final String? clientIde;
final String? enabledFeatures;

final File sessionFile;

Expand All @@ -43,7 +42,6 @@ class UserProperty {
required this.hostOsVersion,
required this.locale,
required this.clientIde,
required this.enabledFeatures,
required this.sessionFile,
});

Expand Down Expand Up @@ -157,6 +155,5 @@ class UserProperty {
'host_os_version': hostOsVersion,
'locale': locale,
'client_ide': clientIde,
'enabled_features': enabledFeatures,
};
}
6 changes: 5 additions & 1 deletion pkgs/unified_analytics/lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,17 @@ Map<String, Object?> generateRequestBody({
required DashEvent eventName,
required Map<String, Object?> eventData,
required UserProperty userProperty,
required String? enabledFeatures,
}) =>
<String, Object?>{
'client_id': clientId,
'events': <Map<String, Object?>>[
<String, Object?>{
'name': eventName.label,
'params': eventData,
'params': {
...eventData,
if (enabledFeatures != null) 'enabled_features': enabledFeatures,
},
}
],
'user_properties': userProperty.preparePayload()
Expand Down
40 changes: 30 additions & 10 deletions pkgs/unified_analytics/test/unified_analytics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ void main() {
# All other lines are configuration lines. They have
# the form "name=value". If multiple lines contain
# the same configuration name with different values,
# the parser will default to a conservative value.
# the parser will default to a conservative value.
# DISABLING TELEMETRY REPORTING
#
Expand Down Expand Up @@ -548,7 +548,7 @@ reporting=1
# All other lines are configuration lines. They have
# the form "name=value". If multiple lines contain
# the same configuration name with different values,
# the parser will default to a conservative value.
# the parser will default to a conservative value.
# DISABLING TELEMETRY REPORTING
#
Expand Down Expand Up @@ -632,7 +632,7 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion
);
});

test('Check that UserProperty class has all the necessary keys', () {
test('The UserProperty class has all the necessary keys', () {
const userPropertyKeys = <String>[
'session_id',
'flutter_channel',
Expand All @@ -645,7 +645,6 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion
'host_os_version',
'locale',
'client_ide',
'enabled_features',
];
expect(analytics.userPropertyMap.keys.length, userPropertyKeys.length,
reason: 'There should only be ${userPropertyKeys.length} keys');
Expand Down Expand Up @@ -826,15 +825,14 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion
eventName: DashEvent.hotReloadTime,
eventData: eventData,
userProperty: analytics.userProperty,
enabledFeatures: 'enable-native-assets',
);

// Checks for the top level keys
expect(body.containsKey('client_id'), true,
reason: '"client_id" is required at the top level');
expect(body.containsKey('events'), true,
reason: '"events" is required at the top level');
expect(body.containsKey('user_properties'), true,
reason: '"user_properties" is required at the top level');

// Regex for the client id
final clientIdPattern = RegExp(
Expand All @@ -846,14 +844,36 @@ ${initialTool.label}=$dateStamp,$toolsMessageVersion
expect(clientIdPattern.hasMatch(body['client_id'] as String), true,
reason: 'The client id is not properly formatted, ie '
'46cc0ba6-f604-4fd9-aa2f-8a20beb24cd4');
expect(
(body['events'][0] as Map<String, dynamic>).containsKey('name'), true,
expect(body['events'][0] as Map<String, Object?>, contains('name'),
reason: 'Each event in the events array needs a name');
expect(
(body['events'][0] as Map<String, dynamic>).containsKey('params'), true,
expect(body['events'][0] as Map<String, Object?>, contains('params'),
reason: 'Each event in the events array needs a params key');
});

test(
'The list of enabled features is included as an event parameter in every sent event',
() {
final eventData = <String, dynamic>{
'time': 5,
'command': 'run',
};

final Map<String, dynamic> body = generateRequestBody(
clientId: Uuid().generateV4(),
eventName: DashEvent.hotReloadTime,
eventData: eventData,
userProperty: analytics.userProperty,
enabledFeatures: 'enable-native-assets',
);

expect((body['events'][0] as Map<String, Object?>)['params'],
contains('enabled_features'));
expect(
(body['events'][0] as Map<String, dynamic>)['params']
['enabled_features'],
'enable-native-assets');
});

test('Check that log file is correctly persisting events sent', () {
final int numberOfEvents = max((kLogFileLength * 0.1).floor(), 5);

Expand Down

0 comments on commit 3934675

Please sign in to comment.