From 5f5a0b56c1f8d4176d78640a437728c4da9ea864 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Thu, 9 Jan 2025 16:01:04 -0800 Subject: [PATCH 1/4] [Inspector V2] Fix inspector analytics (#8684) --- .../inspector/inspector_controller.dart | 10 ++++++++++ .../inspector/inspector_screen_body.dart | 13 ++++++++++-- .../inspector/inspector_tree_controller.dart | 15 ++++++++++++-- .../inspector_settings_dialog.dart | 2 +- .../inspector_v2/inspector_controller.dart | 14 ++++++++++++- .../inspector_v2/inspector_screen_body.dart | 7 ++++++- .../inspector_tree_controller.dart | 13 ++++++++++-- .../src/shared/analytics/_analytics_web.dart | 20 +++++++++++++------ .../lib/src/shared/analytics/constants.dart | 2 ++ .../lib/src/shared/analytics/metrics.dart | 12 +++++------ 10 files changed, 87 insertions(+), 21 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart index f9d1f866c1c..abbc177e71b 100644 --- a/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector/inspector_controller.dart @@ -23,6 +23,9 @@ import 'package:logging/logging.dart'; import 'package:vm_service/vm_service.dart'; import '../../service/service_extensions.dart' as extensions; +import '../../shared/analytics/analytics.dart' as ga; +import '../../shared/analytics/constants.dart' as gac; +import '../../shared/analytics/metrics.dart'; import '../../shared/console/eval/inspector_tree.dart'; import '../../shared/console/primitives/simple_items.dart'; import '../../shared/diagnostics/diagnostics_node.dart'; @@ -666,6 +669,13 @@ class InspectorController extends DisposableController subtreeRoot = newSelection; applyNewSelection(newSelection, detailsSelection, true); + + // Send an event that a widget was selected on the device. + ga.select( + gac.inspector, + gac.onDeviceSelection, + screenMetricsProvider: () => InspectorScreenMetrics.legacy(), + ); } catch (error, st) { if (selectionGroups.next == group) { _log.shout(error, error, st); diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart index bbf47db4dd7..c737f4e42c6 100644 --- a/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart +++ b/packages/devtools_app/lib/src/screens/inspector/inspector_screen_body.dart @@ -13,6 +13,7 @@ import '../../service/service_extension_widgets.dart'; import '../../service/service_extensions.dart' as extensions; import '../../shared/analytics/analytics.dart' as ga; import '../../shared/analytics/constants.dart' as gac; +import '../../shared/analytics/metrics.dart'; import '../../shared/console/eval/inspector_tree.dart'; import '../../shared/globals.dart'; import '../../shared/managers/error_badge_manager.dart'; @@ -265,7 +266,11 @@ class InspectorScreenBodyState extends State } void _refreshInspector() { - ga.select(gac.inspector, gac.refresh); + ga.select( + gac.inspector, + gac.refresh, + screenMetricsProvider: () => InspectorScreenMetrics.legacy(), + ); unawaited( blockWhileInProgress(() async { // If the user is force refreshing the inspector before the first load has @@ -275,7 +280,11 @@ class InspectorScreenBodyState extends State // We do not want to complete this timing operation because the force // refresh will skew the results. ga.cancelTimingOperation(InspectorScreen.id, gac.pageReady); - ga.select(gac.inspector, gac.refreshEmptyTree); + ga.select( + gac.inspector, + gac.refreshEmptyTree, + screenMetricsProvider: () => InspectorScreenMetrics.legacy(), + ); controller.firstInspectorTreeLoadCompleted = true; } await controller.onForceRefresh(); diff --git a/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart b/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart index 0448d09d33b..dcaaebe7148 100644 --- a/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector/inspector_tree_controller.dart @@ -447,7 +447,11 @@ class InspectorTreeController extends DisposableController void onSelectNode(InspectorTreeNode? node) { selection = node; - ga.select(gac.inspector, gac.treeNodeSelection); + ga.select( + gac.inspector, + gac.treeNodeSelection, + screenMetricsProvider: () => InspectorScreenMetrics.legacy(), + ); expandPath(node); } @@ -1002,7 +1006,14 @@ class _InspectorTreeState extends State if (!controller.firstInspectorTreeLoadCompleted && widget.isSummaryTree) { final screenId = widget.screenId; if (screenId != null) { - ga.timeEnd(screenId, gac.pageReady); + ga.timeEnd( + screenId, + gac.pageReady, + screenMetricsProvider: + () => InspectorScreenMetrics.legacy( + rowCount: treeControllerLocal.numRows, + ), + ); unawaited( serviceConnection.sendDwdsEvent( screen: screenId, diff --git a/packages/devtools_app/lib/src/screens/inspector_shared/inspector_settings_dialog.dart b/packages/devtools_app/lib/src/screens/inspector_shared/inspector_settings_dialog.dart index 11d8f50aa36..3c793b3903d 100644 --- a/packages/devtools_app/lib/src/screens/inspector_shared/inspector_settings_dialog.dart +++ b/packages/devtools_app/lib/src/screens/inspector_shared/inspector_settings_dialog.dart @@ -75,7 +75,7 @@ class FlutterInspectorSettingsDialog extends StatelessWidget { description: 'Disable the redesigned Flutter inspector. Please know that ' 'the legacy inspector may be removed in a future release.', - gaItem: gac.inspectorV2Enabled, + gaItem: gac.inspectorV2Disabled, ), ), const SizedBox(height: largeSpacing), diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart index 61b9c127b08..0e7255b445e 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_controller.dart @@ -25,6 +25,7 @@ import 'package:vm_service/vm_service.dart'; import '../../service/service_extensions.dart' as extensions; import '../../shared/analytics/analytics.dart' as ga; import '../../shared/analytics/constants.dart' as gac; +import '../../shared/analytics/metrics.dart'; import '../../shared/console/eval/inspector_tree_v2.dart'; import '../../shared/console/primitives/simple_items.dart'; import '../../shared/diagnostics/diagnostics_node.dart'; @@ -392,7 +393,11 @@ class InspectorController extends DisposableController // We do not want to complete this timing operation because the force // refresh will skew the results. ga.cancelTimingOperation(InspectorScreen.id, gac.pageReady); - ga.select(gac.inspector, gac.refreshEmptyTree); + ga.select( + gac.inspector, + gac.refreshEmptyTree, + screenMetricsProvider: () => InspectorScreenMetrics.v2(), + ); firstInspectorTreeLoadCompleted = true; } await onForceRefresh(); @@ -766,6 +771,13 @@ class InspectorController extends DisposableController selectedNode: newSelection, group: group, ); + + // Send an event that a widget was selected on the device. + ga.select( + gac.inspector, + gac.onDeviceSelection, + screenMetricsProvider: () => InspectorScreenMetrics.v2(), + ); } catch (error, st) { if (selectionGroups.next == group) { _log.shout(error, error, st); diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen_body.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen_body.dart index 7d918ff0a89..233fe5871b4 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen_body.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_screen_body.dart @@ -11,6 +11,7 @@ import 'package:flutter/material.dart'; import '../../shared/analytics/analytics.dart' as ga; import '../../shared/analytics/constants.dart' as gac; +import '../../shared/analytics/metrics.dart'; import '../../shared/console/eval/inspector_tree_v2.dart'; import '../../shared/globals.dart'; import '../../shared/managers/banner_messages.dart'; @@ -219,7 +220,11 @@ class InspectorScreenBodyState extends State } void _refreshInspector() { - ga.select(gac.inspector, gac.refresh); + ga.select( + gac.inspector, + gac.refresh, + screenMetricsProvider: () => InspectorScreenMetrics.v2(), + ); unawaited( blockWhileInProgress(() async { await controller.refreshInspector(); diff --git a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart index 56784743d5a..d1891329231 100644 --- a/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart +++ b/packages/devtools_app/lib/src/screens/inspector_v2/inspector_tree_controller.dart @@ -553,7 +553,11 @@ class InspectorTreeController extends DisposableController void onSelectNode(InspectorTreeNode? node) { setSelectedNode(node, notifyFlutterInspector: true); - ga.select(gac.inspector, gac.treeNodeSelection); + ga.select( + gac.inspector, + gac.treeNodeSelection, + screenMetricsProvider: () => InspectorScreenMetrics.v2(), + ); final diagnostic = node?.diagnostic; if (diagnostic != null && diagnostic.groupIsHidden) { diagnostic.hideableGroupLeader?.toggleHiddenGroup(); @@ -1136,7 +1140,12 @@ class _InspectorTreeState extends State if (!controller.firstInspectorTreeLoadCompleted) { final screenId = widget.screenId; if (screenId != null) { - ga.timeEnd(screenId, gac.pageReady); + ga.timeEnd( + screenId, + gac.pageReady, + screenMetricsProvider: + () => InspectorScreenMetrics.v2(rowCount: rows.length), + ); unawaited( serviceConnection.sendDwdsEvent( screen: screenId, diff --git a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart index 178e0817816..2154cba2421 100644 --- a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart +++ b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart @@ -67,6 +67,8 @@ extension type GtagEventDevTools._(JSObject _) implements GtagEvent { // NOTE: Do not reorder any of these. Order here must match the order in the // Google Analytics console. + // IMPORTANT! Only string and int values are supported. All other value + // types will be ignored in GA4. String? user_app, // dimension1 (flutter or web) String? user_build, // dimension2 (debug or profile) String? user_platform, // dimension3 (android/ios/fuchsia/linux/mac/windows) @@ -112,7 +114,7 @@ extension type GtagEventDevTools._(JSObject _) implements GtagEvent { String? android_app_id, //metric13 String? ios_bundle_id, //metric14 // Inspector screen metrics. See [InspectorScreenMetrics]. - bool? is_v2_inspector, // metric15 + String? is_v2_inspector, // metric15 }); factory GtagEventDevTools._create({ @@ -207,7 +209,9 @@ extension type GtagEventDevTools._(JSObject _) implements GtagEvent { : null, // [InspectorScreenMetrics] is_v2_inspector: - screenMetrics is InspectorScreenMetrics ? screenMetrics.isV2 : null, + screenMetrics is InspectorScreenMetrics + ? screenMetrics.isV2.toString() + : null, ); } @@ -243,7 +247,7 @@ extension type GtagEventDevTools._(JSObject _) implements GtagEvent { external int? get inspector_tree_controller_id; external String? get android_app_id; external String? get ios_bundle_id; - external bool? get is_v2_inspector; + external String? get is_v2_inspector; } extension type GtagExceptionDevTools._(JSObject _) implements GtagException { @@ -254,6 +258,8 @@ extension type GtagExceptionDevTools._(JSObject _) implements GtagException { // NOTE: Do not reorder any of these. Order here must match the order in the // Google Analytics console. + // IMPORTANT! Only string and int values are supported. All other value + // types will be ignored in GA4. String? user_app, // dimension1 (flutter or web) String? user_build, // dimension2 (debug or profile) String? user_platform, // dimension3 (android or ios) @@ -298,7 +304,7 @@ extension type GtagExceptionDevTools._(JSObject _) implements GtagException { String? android_app_id, //metric13 String? ios_bundle_id, //metric14 // Inspector screen metrics. See [InspectorScreenMetrics]. - bool? is_v2_inspector, // metric15 + String? is_v2_inspector, // metric15 }); factory GtagExceptionDevTools._create( @@ -385,7 +391,9 @@ extension type GtagExceptionDevTools._(JSObject _) implements GtagException { : null, // [InspectorScreenMetrics] is_v2_inspector: - screenMetrics is InspectorScreenMetrics ? screenMetrics.isV2 : null, + screenMetrics is InspectorScreenMetrics + ? screenMetrics.isV2.toString() + : null, ); } @@ -1026,7 +1034,7 @@ final class _DevToolsEventMetrics extends ua.CustomMetrics { final int? rootSetCount; final int? rowCount; final int? inspectorTreeControllerId; - final bool? isV2Inspector; + final String? isV2Inspector; // [DeepLinkScreenMetrics] final String? androidAppId; diff --git a/packages/devtools_app/lib/src/shared/analytics/constants.dart b/packages/devtools_app/lib/src/shared/analytics/constants.dart index 83679f50f6b..a3a714ffaa1 100644 --- a/packages/devtools_app/lib/src/shared/analytics/constants.dart +++ b/packages/devtools_app/lib/src/shared/analytics/constants.dart @@ -84,6 +84,7 @@ const selectWidgetMode = 'selectWidgetMode'; const enableOnDeviceInspector = 'enableOnDeviceInspector'; const showOnDeviceInspector = 'showInspector'; const treeNodeSelection = 'treeNodeSelection'; +const onDeviceSelection = 'onDeviceSelection'; const inspectorSettings = 'inspectorSettings'; const loggingSettings = 'loggingSettings'; const refreshPubRoots = 'refreshPubRoots'; @@ -119,6 +120,7 @@ const wasm = 'wasm'; const verboseLogging = 'verboseLogging'; const inspectorHoverEvalMode = 'inspectorHoverEvalMode'; const inspectorV2Enabled = 'inspectorV2Enabled'; +const inspectorV2Disabled = 'inspectorV2Disabled'; const inspectorAutoRefreshEnabled = 'inspectorAutoRefreshEnabled'; const inspectorV2Docs = 'inspectorV2Docs'; const clearLogs = 'clearLogs'; diff --git a/packages/devtools_app/lib/src/shared/analytics/metrics.dart b/packages/devtools_app/lib/src/shared/analytics/metrics.dart index dc4ef4ed74b..a3e1b9d96ce 100644 --- a/packages/devtools_app/lib/src/shared/analytics/metrics.dart +++ b/packages/devtools_app/lib/src/shared/analytics/metrics.dart @@ -64,15 +64,15 @@ class ProfilerScreenMetrics extends ScreenAnalyticsMetrics { class InspectorScreenMetrics extends ScreenAnalyticsMetrics { InspectorScreenMetrics.legacy({ - required this.rootSetCount, - required this.rowCount, - required this.inspectorTreeControllerId, + this.rootSetCount, + this.rowCount, + this.inspectorTreeControllerId, }) : isV2 = false; InspectorScreenMetrics.v2({ - required this.rootSetCount, - required this.rowCount, - required this.inspectorTreeControllerId, + this.rootSetCount, + this.rowCount, + this.inspectorTreeControllerId, }) : isV2 = true; static const summaryTreeGaId = 0; From ea066c43383e6c2866acd1c512d781eb77cb332f Mon Sep 17 00:00:00 2001 From: Kenzie Davisson <43759233+kenzieschmoll@users.noreply.github.com> Date: Fri, 10 Jan 2025 13:50:22 -0800 Subject: [PATCH 2/4] Optimize stack traces for analytics (#8687) --- .../src/shared/analytics/_analytics_stub.dart | 3 +- .../src/shared/analytics/_analytics_web.dart | 24 ++-- .../shared/analytics/analytics_common.dart | 134 ++++++++++++++++++ .../shared/framework/app_error_handling.dart | 18 +-- .../lib/src/shared/primitives/utils.dart | 7 +- .../test/shared/analytics/analytics_test.dart | 95 +++++++++++++ .../test/shared/primitives/utils_test.dart | 8 +- 7 files changed, 255 insertions(+), 34 deletions(-) create mode 100644 packages/devtools_app/test/shared/analytics/analytics_test.dart diff --git a/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart b/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart index 6392dd76833..fe8256dfe5e 100644 --- a/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart +++ b/packages/devtools_app/lib/src/shared/analytics/_analytics_stub.dart @@ -9,6 +9,7 @@ import 'dart:async'; import 'package:logging/logging.dart'; +import 'package:stack_trace/stack_trace.dart' as stack_trace; import 'analytics_common.dart'; @@ -94,7 +95,7 @@ void impression( void reportError( String errorMessage, { - List stackTraceSubstrings = const [], + stack_trace.Trace? stackTrace, bool fatal = false, }) {} diff --git a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart index 2154cba2421..fe9f059f94d 100644 --- a/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart +++ b/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart @@ -13,13 +13,13 @@ import 'dart:js_interop'; import 'package:devtools_app_shared/ui.dart'; import 'package:flutter/foundation.dart'; import 'package:logging/logging.dart'; +import 'package:stack_trace/stack_trace.dart' as stack_trace; import 'package:unified_analytics/unified_analytics.dart' as ua; import 'package:web/web.dart'; import '../globals.dart'; import '../managers/dtd_manager_extensions.dart'; import '../primitives/query_parameters.dart'; -import '../primitives/utils.dart'; import '../server/server.dart' as server; import '../utils/utils.dart'; import 'analytics_common.dart'; @@ -681,7 +681,7 @@ String? _lastGaError; /// chunks to GA4 through unified_analytics. void reportError( String errorMessage, { - List stackTraceSubstrings = const [], + stack_trace.Trace? stackTrace, bool fatal = false, }) { // Don't keep recording same last error. @@ -690,14 +690,14 @@ void reportError( final gTagExceptionWithStackTrace = GtagExceptionDevTools._create( // Include the stack trace in the message for legacy analytics. - '$errorMessage\n${stackTraceSubstrings.join()}', + '$errorMessage\n${stackTrace?.toString() ?? ''}', fatal: fatal, ); GTag.exception(gaExceptionProvider: () => gTagExceptionWithStackTrace); final uaEvent = _uaEventFromGtagException( GtagExceptionDevTools._create(errorMessage, fatal: fatal), - stackTraceSubstrings: stackTraceSubstrings, + stackTrace: stackTrace, ); unawaited(dtdManager.sendAnalyticsEvent(uaEvent)); } @@ -958,25 +958,17 @@ ua.Event _uaEventFromGtagEvent(GtagEventDevTools gtagEvent) { ua.Event _uaEventFromGtagException( GtagExceptionDevTools gtagException, { - List stackTraceSubstrings = const [], + stack_trace.Trace? stackTrace, }) { + final stackTraceAsMap = createStackTraceForAnalytics(stackTrace); + // Any data entries that have a null value will be removed from the event data // in the [ua.Event.exception] constructor. return ua.Event.exception( exception: gtagException.description ?? 'unknown exception', data: { 'fatal': gtagException.fatal, - // Each stack trace substring of length [ga4ParamValueCharacterLimit] - // contains information for ~1 stack frame, so including 8 chunks should - // give us enough information to understand the source of the exception. - 'stackTraceChunk0': stackTraceSubstrings.safeGet(0), - 'stackTraceChunk1': stackTraceSubstrings.safeGet(1), - 'stackTraceChunk2': stackTraceSubstrings.safeGet(2), - 'stackTraceChunk3': stackTraceSubstrings.safeGet(3), - 'stackTraceChunk4': stackTraceSubstrings.safeGet(4), - 'stackTraceChunk5': stackTraceSubstrings.safeGet(5), - 'stackTraceChunk6': stackTraceSubstrings.safeGet(6), - 'stackTraceChunk7': stackTraceSubstrings.safeGet(7), + ...stackTraceAsMap, 'userApp': gtagException.user_app, 'userBuild': gtagException.user_build, 'userPlatform': gtagException.user_platform, diff --git a/packages/devtools_app/lib/src/shared/analytics/analytics_common.dart b/packages/devtools_app/lib/src/shared/analytics/analytics_common.dart index 60724bb1387..af0569f9e2a 100644 --- a/packages/devtools_app/lib/src/shared/analytics/analytics_common.dart +++ b/packages/devtools_app/lib/src/shared/analytics/analytics_common.dart @@ -5,6 +5,12 @@ // Code in this file should be able to be imported by both web and dart:io // dependent libraries. +import 'package:collection/collection.dart'; +import 'package:flutter/widgets.dart'; +import 'package:stack_trace/stack_trace.dart' as stack_trace; + +import '../primitives/utils.dart'; + /// Base class for all screen metrics classes. /// /// Create a subclass of this class to store custom metrics for a screen. All @@ -26,3 +32,131 @@ abstract class ScreenAnalyticsMetrics {} /// The character limit for each event parameter value sent to GA4. const ga4ParamValueCharacterLimit = 100; + +/// Returns a stack trace as a [Map] for consumption by GA4 analytics. +/// +/// The returned [Map] is indexed into [stackTraceChunksLimit] chunks, where +/// each chunk is a substring of length [ga4ParamValueCharacterLimit]. Each +/// substring contains information for ~1 stack frame, so including +/// [stackTraceChunksLimit] chunks should give us enough information to +/// understand the source of the exception. +/// +/// This method uses a heuristic to attempt to include a minimal amount of +/// DevTools-related information in each stack trace. However, there is no +/// guarantee that the returned stack trace will contain any DevTools +/// information. For example, this may happen if all stack frames in the stack +/// trace are from the Flutter framework or from some other package. +Map createStackTraceForAnalytics( + stack_trace.Trace? stackTrace, +) { + if (stackTrace == null) return {}; + + // Consider a stack frame that contains the 'devtools' String to be from one + // of the DevTools packages (devtools_app, devtools_shared, etc.). + const devToolsIdentifier = 'devtools'; + const stackTraceChunksLimit = 10; + const maxCharacterLimit = stackTraceChunksLimit * ga4ParamValueCharacterLimit; + + // Reduce whitespace characters to optimize available space. + final trimmedStackFrames = + stackTrace.frames.map((f) => '${f.location} | ${f.member}\n').toList(); + final stackTraceAsString = trimmedStackFrames.join(); + + var stackTraceChunksForGa = chunkForGa( + stackTraceAsString, + chunkCountLimit: stackTraceChunksLimit, + ); + + // Count the number of stack frames that fully fit within [maxCharacterLimit]. + final framesThatFitCount = countFullFramesThatFit( + trimmedStackFrames, + maxCharacterLimit: maxCharacterLimit, + ); + final framesThatFit = trimmedStackFrames.sublist(0, framesThatFitCount); + + final containsDevToolsFrame = framesThatFit.join().contains( + devToolsIdentifier, + ); + // If the complete stack frames in [stackTraceChunksForGa] do not contain any + // DevTools data, modify the stack trace to add DevTools information that may + // help with debugging the exception. + if (!containsDevToolsFrame) { + final devToolsFrames = trimmedStackFrames + .where((entry) => entry.contains(devToolsIdentifier)) + .toList() + .safeSublist(0, 3); + if (devToolsFrames.isNotEmpty) { + const modifierLine = '\n'; + final devToolsFramesCharacterLength = devToolsFrames.fold( + 0, + (sum, frame) => sum += frame.length, + ); + final originalStackTraceCharLimit = + maxCharacterLimit - + devToolsFramesCharacterLength - + modifierLine.length; + final originalFramesThatFitCount = countFullFramesThatFit( + trimmedStackFrames, + maxCharacterLimit: originalStackTraceCharLimit, + ); + + final modifiedStackFrames = [ + ...trimmedStackFrames.sublist(0, originalFramesThatFitCount), + modifierLine, + ...devToolsFrames, + ]; + stackTraceChunksForGa = chunkForGa( + modifiedStackFrames.join(), + chunkCountLimit: stackTraceChunksLimit, + ); + } + } + + final stackTraceChunks = { + 'stackTraceChunk0': stackTraceChunksForGa.safeGet(0), + 'stackTraceChunk1': stackTraceChunksForGa.safeGet(1), + 'stackTraceChunk2': stackTraceChunksForGa.safeGet(2), + 'stackTraceChunk3': stackTraceChunksForGa.safeGet(3), + 'stackTraceChunk4': stackTraceChunksForGa.safeGet(4), + 'stackTraceChunk5': stackTraceChunksForGa.safeGet(5), + 'stackTraceChunk6': stackTraceChunksForGa.safeGet(6), + 'stackTraceChunk7': stackTraceChunksForGa.safeGet(7), + 'stackTraceChunk8': stackTraceChunksForGa.safeGet(8), + 'stackTraceChunk9': stackTraceChunksForGa.safeGet(9), + }; + assert(stackTraceChunks.length == stackTraceChunksLimit); + return stackTraceChunks; +} + +/// Returns the number of stack frames from [stackFrameStrings] that fit within +/// [maxCharacterLimit]. +int countFullFramesThatFit( + List stackFrameStrings, { + required int maxCharacterLimit, +}) { + var count = 0; + var characterCount = 0; + for (final stackFrameAsString in stackFrameStrings) { + characterCount += stackFrameAsString.length; + if (characterCount < maxCharacterLimit) { + count++; + } else { + break; + } + } + return count; +} + +/// Splits [value] up into substrings of size [ga4ParamValueCharacterLimit] so +/// that the data can be set to GA4 through unified_analytics. +/// +/// This will return a [List] up to size [chunkCountLimit] at a maximum. +List chunkForGa(String value, {required int chunkCountLimit}) { + return value + .trim() + .characters + .slices(ga4ParamValueCharacterLimit) + .map((slice) => slice.join()) + .toList() + .safeSublist(0, chunkCountLimit); +} diff --git a/packages/devtools_app/lib/src/shared/framework/app_error_handling.dart b/packages/devtools_app/lib/src/shared/framework/app_error_handling.dart index 616e07a95b7..b45a7b90c40 100644 --- a/packages/devtools_app/lib/src/shared/framework/app_error_handling.dart +++ b/packages/devtools_app/lib/src/shared/framework/app_error_handling.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'dart:convert'; -import 'package:collection/collection.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'package:http/http.dart'; @@ -15,7 +14,6 @@ import 'package:source_maps/source_maps.dart'; import 'package:stack_trace/stack_trace.dart' as stack_trace; import '../analytics/analytics.dart' as ga; -import '../analytics/analytics_common.dart'; import '../globals.dart'; final _log = Logger('app_error_handling'); @@ -95,22 +93,12 @@ Future _reportError( bool notifyUser = false, StackTrace? stack, }) async { - final stackTrace = await _mapAndTersify(stack); + final stackTrace = await _sourceMapStackTrace(stack); final terseStackTrace = stackTrace?.terse; final errorMessageWithTerseStackTrace = '$error\n${terseStackTrace ?? ''}'; _log.severe('[$errorType]: $errorMessageWithTerseStackTrace', error, stack); - // Split the stack trace up into substrings of size - // [ga4ParamValueCharacterLimit] so that we can send the stack trace in chunks - // to GA4 through unified_analytics. - final stackTraceSubstrings = - stackTrace - .toString() - .characters - .slices(ga4ParamValueCharacterLimit) - .map((slice) => slice.join()) - .toList(); - ga.reportError('$error', stackTraceSubstrings: stackTraceSubstrings); + ga.reportError('$error', stackTrace: stackTrace); // Show error message in a notification pop-up: if (notifyUser) { @@ -147,7 +135,7 @@ Future _initializeSourceMapping() async { } } -Future _mapAndTersify(StackTrace? stack) async { +Future _sourceMapStackTrace(StackTrace? stack) async { final originalStackTrace = stack; if (originalStackTrace == null) return null; diff --git a/packages/devtools_app/lib/src/shared/primitives/utils.dart b/packages/devtools_app/lib/src/shared/primitives/utils.dart index 50d3ca2caa6..536bc730ea4 100644 --- a/packages/devtools_app/lib/src/shared/primitives/utils.dart +++ b/packages/devtools_app/lib/src/shared/primitives/utils.dart @@ -633,10 +633,15 @@ String toStringAsFixed(double num, [int fractionDigit = 1]) { return num.toStringAsFixed(fractionDigit); } -extension SafeAccessList on List { +extension SafeListOperations on List { T? safeGet(int index) => index < 0 || index >= length ? null : this[index]; T? safeRemoveLast() => isNotEmpty ? removeLast() : null; + + List safeSublist(int start, [int? end]) { + if (start >= length || start >= (end ?? length)) return []; + return sublist(max(start, 0), min(length, end ?? length)); + } } extension SafeAccess on Iterable { diff --git a/packages/devtools_app/test/shared/analytics/analytics_test.dart b/packages/devtools_app/test/shared/analytics/analytics_test.dart new file mode 100644 index 00000000000..85587db88ab --- /dev/null +++ b/packages/devtools_app/test/shared/analytics/analytics_test.dart @@ -0,0 +1,95 @@ +// Copyright 2025 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'package:devtools_app/src/shared/analytics/analytics_common.dart'; +import 'package:stack_trace/stack_trace.dart' as stack_trace; +import 'package:test/test.dart'; + +void main() { + group('createStackTraceForAnalytics for stack trace', () { + test('with DevTools stack frames near the top', () { + final stackTrace = stack_trace.Trace.parse( + '''file:///b/s/w/ir/x/w/rc/tmp6qd7qvz0/hosted/pub.dev/vm_service-14.3.0/lib/src/vm_service.dart 95:18 28240 +file:///b/s/w/ir/x/w/rc/tmp6qd7qvz0/hosted/pub.dev/vm_service-14.3.0/lib/src/dart_io_extensions.dart 63:12 28301 +file:///b/s/w/ir/x/w/rc/tmp6qd7qvz0/hosted/pub.dev/vm_service-14.3.0/lib/src/dart_io_extensions.dart 61:25 28300 +file:///b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/screens/network/network_service.dart 104:34 28297 +file:///b/s/w/ir/x/w/devtools/packages/devtools_app_shared/lib/src/service/service_utils.dart 82:25 9696 +org-dartlang-sdk:///dart-sdk/lib/_internal/wasm/lib/async_patch.dart 103:30 2140''', + ); + final stackTraceChunks = createStackTraceForAnalytics(stackTrace); + expect( + stackTraceChunks.display, + '''/b/s/w/ir/x/w/rc/tmp6qd7qvz0/hosted/pub.dev/vm_service-14.3.0/lib/src/vm_service.dart 95:18 | 28240 +/b/s/w/ir/x/w/rc/tmp6qd7qvz0/hosted/pub.dev/vm_service-14.3.0/lib/src/dart_io_extensions.dart 63:12 | 28301 +/b/s/w/ir/x/w/rc/tmp6qd7qvz0/hosted/pub.dev/vm_service-14.3.0/lib/src/dart_io_extensions.dart 61:25 | 28300 +/b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/screens/network/network_service.dart 104:34 | 28297 +/b/s/w/ir/x/w/devtools/packages/devtools_app_shared/lib/src/service/service_utils.dart 82:25 | 9696 +org-dartlang-sdk:///dart-sdk/lib/_internal/wasm/lib/async_patch.dart 103:30 | 2140''', + ); + }); + + test('with DevTools stack frames near the bottom', () { + final stackTrace = stack_trace.Trace.parse( + '''file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/box.dart 2212:22 size +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 298:21 performLayout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/shifted_box.dart 239:12 performLayout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 117:21 performLayout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 117:21 performLayout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 117:21 performLayout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 117:21 performLayout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart 210:29 24070 +file:///b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart 557:25 24080 +file:///b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart 549:14 24079 +file:///b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart 207:20 24074''', + ); + final stackTraceChunks = createStackTraceForAnalytics(stackTrace); + expect( + stackTraceChunks.display, + '''/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/box.dart 2212:22 | size +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 298:21 | performLayout +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 | layout +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/shifted_box.dart 239:12 | performLayout +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 | layout +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 117:21 | performLayout + +/b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/screens/performance/panes/timeline_events/timeline_events_controller.dart 210:29 | 24070 +/b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart 557:25 | 24080 +/b/s/w/ir/x/w/devtools/packages/devtools_app/lib/src/shared/analytics/_analytics_web.dart 549:14 | 24079''', // nullnullnull expected since the last 3 chunks do not exist + ); + }); + + test('without DevTools stack frames', () { + final stackTrace = stack_trace.Trace.parse( + ''' +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/box.dart 2212:22 size +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 298:21 performLayout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 layout +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/layout_helper.dart 61:11 layoutChild +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/stack.dart 601:43 _computeSize +file:///b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/stack.dart 628:12 performLayout''', + ); + final stackTraceChunks = createStackTraceForAnalytics(stackTrace); + expect( + stackTraceChunks.display, + '''/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/box.dart 2212:22 | size +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/proxy_box.dart 298:21 | performLayout +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/object.dart 2627:7 | layout +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/layout_helper.dart 61:11 | layoutChild +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/stack.dart 601:43 | _computeSize +/b/s/w/ir/x/w/rc/flutter/packages/flutter/lib/src/rendering/stack.dart 628:12 | performLayout''', + ); + }); + }); +} + +extension on Map { + String get display => values.whereType().join(); +} diff --git a/packages/devtools_app/test/shared/primitives/utils_test.dart b/packages/devtools_app/test/shared/primitives/utils_test.dart index 31fa5acc968..97f3448227d 100644 --- a/packages/devtools_app/test/shared/primitives/utils_test.dart +++ b/packages/devtools_app/test/shared/primitives/utils_test.dart @@ -687,7 +687,7 @@ void main() { }); }); - group('SafeAccess', () { + group('SafeListOperations', () { test('safeFirst', () { final list = []; final iterable = list; @@ -727,6 +727,12 @@ void main() { expect(list.safeRemoveLast(), 1); expect(list.safeRemoveLast(), isNull); }); + + test('safeSublist', () { + expect([1, 2, 3].safeSublist(-1, 2), [1, 2]); + expect([1, 2, 3].safeSublist(0, 6), [1, 2, 3]); + expect([1, 2, 3].safeSublist(2, 1), []); + }); }); }); From d010c20391c14eabf43b8a8daf0ff38994caa4e8 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Fri, 10 Jan 2025 16:49:51 -0800 Subject: [PATCH 3/4] Prepare cherry-pick release - DevTools 2.42.1 --- packages/devtools_app/lib/devtools.dart | 2 +- packages/devtools_app/pubspec.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/devtools_app/lib/devtools.dart b/packages/devtools_app/lib/devtools.dart index dc1bc0b02be..d806f37a3c9 100644 --- a/packages/devtools_app/lib/devtools.dart +++ b/packages/devtools_app/lib/devtools.dart @@ -10,4 +10,4 @@ /// Note: a regexp in the `dt update-version' command logic matches the constant /// declaration `const version =`. If you change the declaration you must also /// modify the regex in the `dt update-version' command logic. -const version = '2.42.0'; +const version = '2.42.1'; diff --git a/packages/devtools_app/pubspec.yaml b/packages/devtools_app/pubspec.yaml index 249e60c13cd..fa209e6c33f 100644 --- a/packages/devtools_app/pubspec.yaml +++ b/packages/devtools_app/pubspec.yaml @@ -4,7 +4,7 @@ publish_to: none # Note: this version should only be updated by running the 'dt update-version' # command that updates the version here and in 'devtools.dart'. -version: 2.42.0 +version: 2.42.1 repository: https://github.com/flutter/devtools/tree/master/packages/devtools_app From 6a6c3eae867aa0e030a848b757d8d7453c169cc9 Mon Sep 17 00:00:00 2001 From: Elliott Brooks <21270878+elliette@users.noreply.github.com> Date: Tue, 28 Jan 2025 10:59:55 -0800 Subject: [PATCH 4/4] Update devtools.dart --- packages/devtools_app/lib/devtools.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/devtools_app/lib/devtools.dart b/packages/devtools_app/lib/devtools.dart index 8d55d154cb9..c466196c5dc 100644 --- a/packages/devtools_app/lib/devtools.dart +++ b/packages/devtools_app/lib/devtools.dart @@ -10,4 +10,4 @@ /// Note: a regexp in the `dt update-version' command logic matches the constant /// declaration `const version =`. If you change the declaration you must also /// modify the regex in the `dt update-version' command logic. -const version = '2.43.0-dev.0'; \ No newline at end of file +const version = '2.43.0-dev.0';