Reland "Update HTTP Profiling ids to use Strings"
This is a reland of commit e7d8261f9ab2b6d472c49d8364e8510a4c0fc827
Original change's description:
> Update HTTP Profiling ids to use Strings
>
> The internal Dart Ids can be 64 bit integers, and are passed to Dev Tools as integers. This is causing errors fetching requests as the ids can overflow once they get to dev tools.
>
> Interaction between `getHttpProfile` and `getHttpProfileRequest` are already performed in https://dart-review.googlesource.com/c/sdk/+/279122/3/pkg/vm_service/test/get_http_profile_test.dart
>
> https://github.com/flutter/devtools/issues/2860
>
> TEST=The original tests test the surface of the RPCs that are updated here. Any tests that needed tweaking have also been fixed. https://github.com/flutter/devtools/pull/5127 is being submitted to Devtools and G3, and the regressions have been tested against vm_service:10.0.0 and vm_service:11.0.0
> CoreLibraryReviewExempt: Changes are only to http profiling
> Change-Id: Ie560dde7629f91f4221c1fe18d153cd999691086
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279122
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Commit-Queue: Dan Chevalier <danchevalier@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>
> Reviewed-by: Ben Konyi <bkonyi@google.com>
TEST=The original tests test the surface of the RPCs that are updated here. Any tests that needed tweaking have also been fixed. https://github.com/flutter/devtools/pull/5127 is being submitted to Devtools and G3, and the regressions have been tested against vm_service:10.0.0 and vm_service:11.0.0
CoreLibraryReviewExempt: Changes are only to http profiling
Change-Id: I132f30111ca9c4c53dec377f6038453cacfc6243
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/280020
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Dan Chevalier <danchevalier@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7845ceb..d43b9e7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -107,6 +107,11 @@
- Deprecated `NetworkInterface.listSupported`. Has always returned true since
Dart 2.3.
+- Finalize `httpEnableTimelineLogging` parameter name transition from `enable`
+ to `enabled`. See [#43638][].
+- **Breaking change** [#51035][]:
+ - Update `NetworkProfiling` to accommodate new `String` ids
+ that are introduced in vm_service:11.0.0
#### `dart:js_util`
diff --git a/pkg/vm_service/CHANGELOG.md b/pkg/vm_service/CHANGELOG.md
index ca4237d..fcfe3ae 100644
--- a/pkg/vm_service/CHANGELOG.md
+++ b/pkg/vm_service/CHANGELOG.md
@@ -1,5 +1,10 @@
# Changelog
+## 11.0.0
+- Change `HttpProfileRequestRef.id` type from `int` to `String`.
+- Change `SocketStatistic.id` type from `int` to `String`.
+- Change `ext.dart.io.getHttpProfileRequest` `id` parameter type from `int` to `String`.
+- Change `ext.dart.io.socketProfilingEnabled` parameter from 'enable' to 'enabled'.
## 10.1.2
- Fix bug where code would try to call `.toJson()` on `String`s.
diff --git a/pkg/vm_service/lib/src/dart_io_extensions.dart b/pkg/vm_service/lib/src/dart_io_extensions.dart
index a33ff29..7759466 100644
--- a/pkg/vm_service/lib/src/dart_io_extensions.dart
+++ b/pkg/vm_service/lib/src/dart_io_extensions.dart
@@ -77,9 +77,10 @@
/// Warning: The returned [Future] will not complete if the target isolate is paused
/// and will only complete when the isolate is resumed.
@Deprecated('Use httpEnableTimelineLogging instead.')
- Future<Success> setHttpEnableTimelineLogging(String isolateId, bool enable) =>
+ Future<Success> setHttpEnableTimelineLogging(
+ String isolateId, bool enabled) =>
_callHelper('ext.dart.io.setHttpEnableTimelineLogging', isolateId, args: {
- 'enable': enable,
+ 'enabled': enabled,
});
/// The `httpEnableTimelineLogging` RPC is used to set and inspect the value of
@@ -119,7 +120,8 @@
/// The `getHttpProfileRequest` RPC is used to retrieve an instance of
/// [HttpProfileRequest], which includes request and response body data.
- Future<HttpProfileRequest> getHttpProfileRequest(String isolateId, int id) =>
+ Future<HttpProfileRequest> getHttpProfileRequest(
+ String isolateId, String id) =>
_callHelper('ext.dart.io.getHttpProfileRequest', isolateId, args: {
'id': id,
});
@@ -205,7 +207,7 @@
json == null ? null : SocketStatistic._fromJson(json);
/// The unique ID associated with this socket.
- final int id;
+ final String id;
/// The time, in microseconds, that this socket was created.
final int startTime;
@@ -364,7 +366,7 @@
/// The ID associated with this request.
///
/// This ID corresponds to the ID of the timeline event for this request.
- final int id;
+ final String id;
/// The HTTP request method associated with this request.
final String method;
@@ -411,7 +413,7 @@
super._fromJson(json);
HttpProfileRequest({
- required int id,
+ required String id,
required String isolateId,
required String method,
required Uri uri,
diff --git a/pkg/vm_service/pubspec.yaml b/pkg/vm_service/pubspec.yaml
index 6ac39bb..46612ac 100644
--- a/pkg/vm_service/pubspec.yaml
+++ b/pkg/vm_service/pubspec.yaml
@@ -1,5 +1,5 @@
name: vm_service
-version: 10.1.2
+version: 11.0.0
description: >-
A library to communicate with a service implementing the Dart VM
service protocol.
diff --git a/runtime/observatory/tests/service/http_enable_timeline_logging_service_test.dart b/runtime/observatory/tests/service/http_enable_timeline_logging_service_test.dart
index d3b3df8..a178877 100644
--- a/runtime/observatory/tests/service/http_enable_timeline_logging_service_test.dart
+++ b/runtime/observatory/tests/service/http_enable_timeline_logging_service_test.dart
@@ -32,7 +32,7 @@
expect(response['enabled'], false);
response = await isolate
- .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': true});
+ .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': true});
expect(response['type'], 'Success');
response =
@@ -41,7 +41,7 @@
expect(response['enabled'], true);
response = await isolate
- .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': false});
+ .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': false});
expect(response['type'], 'Success');
response =
@@ -52,8 +52,8 @@
(Isolate isolate) async {
// Bad argument.
try {
- await isolate
- .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': 'foo'});
+ await isolate.invokeRpcNoUpgrade(
+ kSetHttpEnableTimelineLogging, {'enabled': 'foo'});
} catch (e) {/* expected */}
// Missing argument.
try {
diff --git a/runtime/observatory/tests/service/network_profiling_test.dart b/runtime/observatory/tests/service/network_profiling_test.dart
index 6770c58..b6e46dd 100644
--- a/runtime/observatory/tests/service/network_profiling_test.dart
+++ b/runtime/observatory/tests/service/network_profiling_test.dart
@@ -130,7 +130,7 @@
stats.forEach((socket) {
expect(socket['address'], contains(localhost));
Expect.type<int>(socket['startTime']);
- Expect.type<int>(socket['id']);
+ Expect.type<String>(socket['id']);
Expect.type<int>(socket['port']);
if (socket['socketType'] == 'tcp') {
expect(socket['writeBytes'], content.length);
diff --git a/runtime/observatory_2/tests/service_2/http_enable_timeline_logging_service_test.dart b/runtime/observatory_2/tests/service_2/http_enable_timeline_logging_service_test.dart
index 1c1b0b0..0e1849e 100644
--- a/runtime/observatory_2/tests/service_2/http_enable_timeline_logging_service_test.dart
+++ b/runtime/observatory_2/tests/service_2/http_enable_timeline_logging_service_test.dart
@@ -32,7 +32,7 @@
expect(response['enabled'], false);
response = await isolate
- .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': true});
+ .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': true});
expect(response['type'], 'Success');
response =
@@ -41,7 +41,7 @@
expect(response['enabled'], true);
response = await isolate
- .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': false});
+ .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enabled': false});
expect(response['type'], 'Success');
response =
@@ -52,8 +52,8 @@
(Isolate isolate) async {
// Bad argument.
try {
- await isolate
- .invokeRpcNoUpgrade(kSetHttpEnableTimelineLogging, {'enable': 'foo'});
+ await isolate.invokeRpcNoUpgrade(
+ kSetHttpEnableTimelineLogging, {'enabled': 'foo'});
} catch (e) {/* expected */}
// Missing argument.
try {
diff --git a/runtime/observatory_2/tests/service_2/network_profiling_test.dart b/runtime/observatory_2/tests/service_2/network_profiling_test.dart
index 8a2053b..6f139a7 100644
--- a/runtime/observatory_2/tests/service_2/network_profiling_test.dart
+++ b/runtime/observatory_2/tests/service_2/network_profiling_test.dart
@@ -130,7 +130,7 @@
stats.forEach((socket) {
expect(socket['address'], contains(localhost));
Expect.type<int>(socket['startTime']);
- Expect.type<int>(socket['id']);
+ Expect.type<String>(socket['id']);
Expect.type<int>(socket['port']);
if (socket['socketType'] == 'tcp') {
expect(socket['writeBytes'], content.length);
diff --git a/sdk/lib/_http/http_impl.dart b/sdk/lib/_http/http_impl.dart
index 97326d9..4b10469 100644
--- a/sdk/lib/_http/http_impl.dart
+++ b/sdk/lib/_http/http_impl.dart
@@ -7,7 +7,7 @@
abstract class HttpProfiler {
static const _kType = 'HttpProfile';
- static final Map<int, _HttpProfileData> _profile = {};
+ static final Map<String, _HttpProfileData> _profile = {};
static _HttpProfileData startRequest(
String method,
@@ -19,7 +19,7 @@
return data;
}
- static _HttpProfileData? getHttpProfileRequest(int id) => _profile[id];
+ static _HttpProfileData? getHttpProfileRequest(String id) => _profile[id];
static void clear() => _profile.clear();
@@ -63,7 +63,7 @@
) {
// Grab the ID from the timeline event so HTTP profile IDs can be matched
// to the timeline.
- id = _timeline.pass();
+ id = _timeline.pass().toString();
requestInProgress = true;
requestStartTimestamp = Timeline.now;
_timeline.start('HTTP CLIENT $method', arguments: {
@@ -261,7 +261,7 @@
bool requestInProgress = true;
bool? responseInProgress;
- late final int id;
+ late final String id;
final String method;
final Uri uri;
diff --git a/sdk/lib/io/network_profiling.dart b/sdk/lib/io/network_profiling.dart
index 79ee3de..84a4d55 100644
--- a/sdk/lib/io/network_profiling.dart
+++ b/sdk/lib/io/network_profiling.dart
@@ -5,8 +5,8 @@
part of dart.io;
// TODO(bkonyi): refactor into io_resource_info.dart
-const int _versionMajor = 1;
-const int _versionMinor = 6;
+const int _versionMajor = 2;
+const int _versionMinor = 0;
const String _tcpSocket = 'tcp';
const String _udpSocket = 'udp';
@@ -68,15 +68,7 @@
responseJson = _setHttpEnableTimelineLogging(parameters);
break;
case _kHttpEnableTimelineLogging:
- if (parameters.containsKey('enabled') ||
- parameters.containsKey('enable')) {
- // TODO(bkonyi): Backwards compatibility.
- // See https://github.com/dart-lang/sdk/issues/43638.
- assert(_versionMajor == 1,
- "'enable' is deprecated and should be removed (See #43638)");
- if (parameters.containsKey('enabled')) {
- parameters['enable'] = parameters['enabled']!;
- }
+ if (parameters.containsKey('enabled')) {
_setHttpEnableTimelineLogging(parameters);
}
responseJson = _getHttpEnableTimelineLogging();
@@ -156,13 +148,13 @@
});
String _setHttpEnableTimelineLogging(Map<String, String> parameters) {
- const String kEnable = 'enable';
- if (!parameters.containsKey(kEnable)) {
- throw _missingArgument(kEnable);
+ const String kEnabled = 'enabled';
+ if (!parameters.containsKey(kEnabled)) {
+ throw _missingArgument(kEnabled);
}
- final enable = parameters[kEnable]!.toLowerCase();
+ final enable = parameters[kEnabled]!.toLowerCase();
if (enable != 'true' && enable != 'false') {
- throw _invalidArgument(kEnable, enable);
+ throw _invalidArgument(kEnabled, enable);
}
HttpClient.enableTimelineLogging = enable == 'true';
return _success();
@@ -172,10 +164,8 @@
if (!parameters.containsKey('id')) {
throw _missingArgument('id');
}
- final id = int.tryParse(parameters['id']!);
- if (id == null) {
- throw _invalidArgument('id', parameters['id']!);
- }
+ String id = parameters['id']!;
+
final request = HttpProfiler.getHttpProfileRequest(id);
if (request == null) {
throw "Unable to find request with id: '$id'";
@@ -215,7 +205,7 @@
static bool get enableSocketProfiling => _enableSocketProfiling;
static bool _enableSocketProfiling = false;
- static Map<int, _SocketStatistic> _idToSocketStatistic = {};
+ static Map<String, _SocketStatistic> _idToSocketStatistic = {};
static String toJson() => json.encode({
'type': _kType,
@@ -232,13 +222,15 @@
static void collectStatistic(int id, _SocketProfileType type,
[dynamic object]) {
+ final idKey = id.toString();
if (!_enableSocketProfiling) {
return;
}
// Skip socket that started before _enableSocketProfiling turned on.
- if (!_idToSocketStatistic.containsKey(id) &&
+ if (!_idToSocketStatistic.containsKey(idKey) &&
type != _SocketProfileType.startTime) return;
- _SocketStatistic stats = _idToSocketStatistic[id] ??= _SocketStatistic(id);
+ _SocketStatistic stats =
+ _idToSocketStatistic[idKey] ??= _SocketStatistic(idKey);
switch (type) {
case _SocketProfileType.startTime:
stats.startTime = Timeline.now;
@@ -304,7 +296,7 @@
/// Socket statistic
class _SocketStatistic {
- final int id;
+ final String id;
int? startTime;
int? endTime;
String? address;
