Skip to content

Commit 3602351

Browse files
bkonyiCommit Queue
bkonyi
authored and
Commit Queue
committed
[ VM Service ] Add support for '--[no-]serve-observatory'
To prepare for the eventual removal of Observatory, we plan on disabling Observatory by default while providing an escape hatch to manually serve the tool for some period of time before completely removing Observatory from the SDK. This change adds flags that can be used to configure whether or not Observatory is served. Currently, '--serve-observatory' is the default behavior, but will be changed to '--no-serve-observatory' once tooling is ready to support the escape hatch behavior. Part of dart-lang/sdk#50233 TEST=run_test.dart Change-Id: Ib6d1e1587d9fbd3c61d4a4c75d90635052835844 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/267720 Reviewed-by: Siva Annamalai <[email protected]> Commit-Queue: Ben Konyi <[email protected]>
1 parent b848912 commit 3602351

File tree

11 files changed

+143
-8
lines changed

11 files changed

+143
-8
lines changed

‎pkg/dartdev/lib/src/commands/run.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,10 @@ class RunCommand extends DartdevCommand {
212212
'functionality. Note: Disabling DDS may break some '
213213
'functionality in IDEs and other tooling.',
214214
defaultsTo: true)
215+
..addFlag('serve-observatory',
216+
hide: !verbose,
217+
help: 'Enable hosting Observatory through the VM Service.',
218+
defaultsTo: true)
215219
..addFlag(
216220
'debug-dds',
217221
hide: true,

‎pkg/dartdev/test/commands/run_test.dart

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ const devToolsMessagePrefix =
1818
'The Dart DevTools debugger and profiler is available at: http://127.0.0.1:';
1919
const dartVMServiceMessagePrefix =
2020
'The Dart VM service is listening on http://127.0.0.1:';
21+
final dartVMServiceRegExp =
22+
RegExp(r'The Dart VM service is listening on (http://127.0.0.1:.*)');
2123
const residentFrontendServerPrefix =
2224
'The Resident Frontend Compiler is listening at 127.0.0.1:';
2325

@@ -577,6 +579,82 @@ void main(List<String> args) => print("$b $args");
577579
skip: Platform.isWindows,
578580
);
579581
});
582+
583+
group('Observatory', () {
584+
void generateServedTest({
585+
required bool serve,
586+
required bool enableAuthCodes,
587+
required bool explicitRun,
588+
required bool withDds,
589+
}) {
590+
test(
591+
'${serve ? 'served by default' : 'not served'} ${enableAuthCodes ? "with" : "without"} '
592+
'auth codes, ${explicitRun ? 'explicit' : 'implicit'} run,${withDds ? ' ' : 'no'} DDS',
593+
() async {
594+
p = project(
595+
mainSrc:
596+
'void main() { print("ready"); int i = 0; while(true) { i++; } }',
597+
);
598+
Process process = await p.start([
599+
if (explicitRun) 'run',
600+
'--enable-vm-service',
601+
if (!withDds) '--no-dds',
602+
if (!enableAuthCodes) '--disable-service-auth-codes',
603+
if (!serve) '--no-serve-observatory',
604+
p.relativeFilePath,
605+
]);
606+
607+
final completer = Completer<void>();
608+
609+
late StreamSubscription sub;
610+
late String uri;
611+
sub = process.stdout.transform(utf8.decoder).listen((event) async {
612+
if (event.contains(dartVMServiceRegExp)) {
613+
uri = dartVMServiceRegExp.firstMatch(event)!.group(1)!;
614+
await sub.cancel();
615+
completer.complete();
616+
}
617+
});
618+
// Wait for process to start.
619+
await completer.future;
620+
final client = HttpClient();
621+
final request = await client.getUrl(Uri.parse(uri));
622+
final response = await request.close();
623+
final content = await response.transform(utf8.decoder).join();
624+
expect(content.contains('Dart VM Observatory'), serve);
625+
if (!serve) {
626+
if (withDds) {
627+
expect(content.contains('DevTools'), true);
628+
} else {
629+
expect(
630+
content,
631+
'This VM does not have a registered Dart '
632+
'Development Service (DDS) instance and is not currently serving '
633+
'Dart DevTools.',
634+
);
635+
}
636+
}
637+
process.kill();
638+
},
639+
);
640+
}
641+
642+
const flags = <bool>[true, false];
643+
for (final serve in flags) {
644+
for (final enableAuthCodes in flags) {
645+
for (final explicitRun in flags) {
646+
for (final withDds in flags) {
647+
generateServedTest(
648+
serve: serve,
649+
enableAuthCodes: enableAuthCodes,
650+
explicitRun: explicitRun,
651+
withDds: withDds,
652+
);
653+
}
654+
}
655+
}
656+
}
657+
});
580658
}
581659

582660
void residentRun() {

‎runtime/bin/dart_embedder_api_impl.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ Dart_Isolate CreateVmServiceIsolate(const IsolateCreationData& data,
108108
config.write_service_info_filename,
109109
/*trace_loading=*/false, config.deterministic,
110110
/*enable_service_port_fallback=*/false,
111-
/*wait_for_dds_to_advertise_service=*/false)) {
111+
/*wait_for_dds_to_advertise_service=*/false,
112+
/*serve_observatory=*/true)) {
112113
*error = Utils::StrDup(bin::VmService::GetErrorMessage());
113114
return nullptr;
114115
}
@@ -144,7 +145,8 @@ Dart_Isolate CreateVmServiceIsolateFromKernel(
144145
config.write_service_info_filename,
145146
/*trace_loading=*/false, config.deterministic,
146147
/*enable_service_port_fallback=*/false,
147-
/*wait_for_dds_to_advertise_service=*/false)) {
148+
/*wait_for_dds_to_advertise_service=*/false,
149+
/*serve_observatory*/ true)) {
148150
*error = Utils::StrDup(bin::VmService::GetErrorMessage());
149151
return nullptr;
150152
}

‎runtime/bin/main.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri,
555555
Options::vm_service_auth_disabled(),
556556
Options::vm_write_service_info_filename(), Options::trace_loading(),
557557
Options::deterministic(), Options::enable_service_port_fallback(),
558-
wait_for_dds_to_advertise_service)) {
558+
wait_for_dds_to_advertise_service, !Options::disable_observatory())) {
559559
*error = Utils::StrDup(VmService::GetErrorMessage());
560560
return NULL;
561561
}

‎runtime/bin/main_options.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,11 @@ bool Options::ParseArguments(int argc,
483483
// a VM flag as disabling DDS changes how we configure the VM service,
484484
// so we don't need to handle that case here.
485485
skipVmOption = true;
486+
} else if (IsOption(argv[i], "serve-observatory")) {
487+
// This flag is currently set by default in vmservice_io.dart, so we
488+
// ignore it. --no-serve-observatory is a VM flag so we don't need to
489+
// handle that case here.
490+
skipVmOption = true;
486491
}
487492
if (!skipVmOption) {
488493
temp_vm_options.AddArgument(argv[i]);

‎runtime/bin/main_options.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ namespace bin {
4848
V(long_ssl_cert_evaluation, long_ssl_cert_evaluation) \
4949
V(bypass_trusting_system_roots, bypass_trusting_system_roots) \
5050
V(delayed_filewatch_callback, delayed_filewatch_callback) \
51-
V(mark_main_isolate_as_system_isolate, mark_main_isolate_as_system_isolate)
51+
V(mark_main_isolate_as_system_isolate, mark_main_isolate_as_system_isolate) \
52+
V(no_serve_observatory, disable_observatory)
5253

5354
// Boolean flags that have a short form.
5455
#define SHORT_BOOL_OPTIONS_LIST(V) \

‎runtime/bin/run_vm_tests.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri,
151151
/*write_service_info_filename*/ "",
152152
/*trace_loading=*/false, /*deterministic=*/true,
153153
/*enable_service_port_fallback=*/false,
154-
/*wait_for_dds_to_advertise_service*/ false)) {
154+
/*wait_for_dds_to_advertise_service*/ false,
155+
/*serve_observatory*/ true)) {
155156
*error = Utils::StrDup(bin::VmService::GetErrorMessage());
156157
return nullptr;
157158
}

‎runtime/bin/vmservice_impl.cc

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,8 @@ bool VmService::Setup(const char* server_ip,
119119
bool trace_loading,
120120
bool deterministic,
121121
bool enable_service_port_fallback,
122-
bool wait_for_dds_to_advertise_service) {
122+
bool wait_for_dds_to_advertise_service,
123+
bool serve_observatory) {
123124
Dart_Isolate isolate = Dart_CurrentIsolate();
124125
ASSERT(isolate != NULL);
125126
SetServerAddress("");
@@ -196,6 +197,10 @@ bool VmService::Setup(const char* server_ip,
196197
Dart_NewBoolean(wait_for_dds_to_advertise_service));
197198
SHUTDOWN_ON_ERROR(result);
198199

200+
result = Dart_SetField(library, DartUtils::NewString("_serveObservatory"),
201+
serve_observatory ? Dart_True() : Dart_False());
202+
SHUTDOWN_ON_ERROR(result);
203+
199204
// Are we running on Windows?
200205
#if defined(DART_HOST_OS_WINDOWS)
201206
Dart_Handle is_windows = Dart_True();

‎runtime/bin/vmservice_impl.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ class VmService {
2222
bool trace_loading,
2323
bool deterministic,
2424
bool enable_service_port_fallback,
25-
bool wait_for_dds_to_advertise_service);
25+
bool wait_for_dds_to_advertise_service,
26+
bool serve_observatory);
2627

2728
static void SetNativeResolver();
2829

‎sdk/lib/_internal/vm/bin/vmservice_io.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ StreamSubscription<ProcessSignal>? _signalSubscription;
3939
bool _enableServicePortFallback = false;
4040
@pragma("vm:entry-point")
4141
bool _waitForDdsToAdvertiseService = false;
42+
@pragma("vm:entry-point")
43+
bool _serveObservatory = true;
4244

4345
// HTTP server.
4446
Server? server;

‎sdk/lib/_internal/vm/bin/vmservice_server.dart

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,43 @@ class Server {
369369
}
370370
return;
371371
}
372-
372+
if (!_serveObservatory) {
373+
final ddsUri = _service.ddsUri;
374+
if (ddsUri == null) {
375+
request.response.headers.contentType = ContentType.text;
376+
request.response.write('This VM does not have a registered Dart '
377+
'Development Service (DDS) instance and is not currently serving '
378+
'Dart DevTools.');
379+
request.response.close();
380+
return;
381+
}
382+
// We build this path manually rather than manipulating ddsUri directly
383+
// as the resulting path requires an unencoded '#'. The Uri class will
384+
// always encode '#' as '%23' in paths to avoid conflicts with fragments,
385+
// which will result in the redirect failing.
386+
final path = StringBuffer();
387+
// Add authentication code to the path.
388+
if (ddsUri.pathSegments.length > 1) {
389+
path.writeAll([
390+
ddsUri.pathSegments
391+
.sublist(0, ddsUri.pathSegments.length - 1)
392+
.join('/'),
393+
'/',
394+
]);
395+
}
396+
final queryComponent = Uri.encodeQueryComponent(
397+
ddsUri.replace(scheme: 'ws', path: '${path}ws').toString(),
398+
);
399+
path.writeAll([
400+
'devtools/#/',
401+
'?uri=$queryComponent',
402+
]);
403+
final redirectUri = Uri.parse(
404+
'http://${ddsUri.host}:${ddsUri.port}/$path',
405+
);
406+
request.response.redirect(redirectUri);
407+
return;
408+
}
373409
if (assets == null) {
374410
request.response.headers.contentType = ContentType.text;
375411
request.response.write('This VM was built without the Observatory UI.');

0 commit comments

Comments
 (0)