Skip to content

Commit

Permalink
[ VM Service ] Add support for '--[no-]serve-observatory'
Browse files Browse the repository at this point in the history
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 #50233

TEST=run_test.dart

Change-Id: Ib6d1e1587d9fbd3c61d4a4c75d90635052835844
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/267720
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
  • Loading branch information
bkonyi authored and Commit Queue committed Nov 21, 2022
1 parent b848912 commit 3602351
Show file tree
Hide file tree
Showing 11 changed files with 143 additions and 8 deletions.
4 changes: 4 additions & 0 deletions pkg/dartdev/lib/src/commands/run.dart
Expand Up @@ -212,6 +212,10 @@ class RunCommand extends DartdevCommand {
'functionality. Note: Disabling DDS may break some '
'functionality in IDEs and other tooling.',
defaultsTo: true)
..addFlag('serve-observatory',
hide: !verbose,
help: 'Enable hosting Observatory through the VM Service.',
defaultsTo: true)
..addFlag(
'debug-dds',
hide: true,
Expand Down
78 changes: 78 additions & 0 deletions pkg/dartdev/test/commands/run_test.dart
Expand Up @@ -18,6 +18,8 @@ const devToolsMessagePrefix =
'The Dart DevTools debugger and profiler is available at: http://127.0.0.1:';
const dartVMServiceMessagePrefix =
'The Dart VM service is listening on http://127.0.0.1:';
final dartVMServiceRegExp =
RegExp(r'The Dart VM service is listening on (http://127.0.0.1:.*)');
const residentFrontendServerPrefix =
'The Resident Frontend Compiler is listening at 127.0.0.1:';

Expand Down Expand Up @@ -577,6 +579,82 @@ void main(List<String> args) => print("$b $args");
skip: Platform.isWindows,
);
});

group('Observatory', () {
void generateServedTest({
required bool serve,
required bool enableAuthCodes,
required bool explicitRun,
required bool withDds,
}) {
test(
'${serve ? 'served by default' : 'not served'} ${enableAuthCodes ? "with" : "without"} '
'auth codes, ${explicitRun ? 'explicit' : 'implicit'} run,${withDds ? ' ' : 'no'} DDS',
() async {
p = project(
mainSrc:
'void main() { print("ready"); int i = 0; while(true) { i++; } }',
);
Process process = await p.start([
if (explicitRun) 'run',
'--enable-vm-service',
if (!withDds) '--no-dds',
if (!enableAuthCodes) '--disable-service-auth-codes',
if (!serve) '--no-serve-observatory',
p.relativeFilePath,
]);

final completer = Completer<void>();

late StreamSubscription sub;
late String uri;
sub = process.stdout.transform(utf8.decoder).listen((event) async {
if (event.contains(dartVMServiceRegExp)) {
uri = dartVMServiceRegExp.firstMatch(event)!.group(1)!;
await sub.cancel();
completer.complete();
}
});
// Wait for process to start.
await completer.future;
final client = HttpClient();
final request = await client.getUrl(Uri.parse(uri));
final response = await request.close();
final content = await response.transform(utf8.decoder).join();
expect(content.contains('Dart VM Observatory'), serve);
if (!serve) {
if (withDds) {
expect(content.contains('DevTools'), true);
} else {
expect(
content,
'This VM does not have a registered Dart '
'Development Service (DDS) instance and is not currently serving '
'Dart DevTools.',
);
}
}
process.kill();
},
);
}

const flags = <bool>[true, false];
for (final serve in flags) {
for (final enableAuthCodes in flags) {
for (final explicitRun in flags) {
for (final withDds in flags) {
generateServedTest(
serve: serve,
enableAuthCodes: enableAuthCodes,
explicitRun: explicitRun,
withDds: withDds,
);
}
}
}
}
});
}

void residentRun() {
Expand Down
6 changes: 4 additions & 2 deletions runtime/bin/dart_embedder_api_impl.cc
Expand Up @@ -108,7 +108,8 @@ Dart_Isolate CreateVmServiceIsolate(const IsolateCreationData& data,
config.write_service_info_filename,
/*trace_loading=*/false, config.deterministic,
/*enable_service_port_fallback=*/false,
/*wait_for_dds_to_advertise_service=*/false)) {
/*wait_for_dds_to_advertise_service=*/false,
/*serve_observatory=*/true)) {
*error = Utils::StrDup(bin::VmService::GetErrorMessage());
return nullptr;
}
Expand Down Expand Up @@ -144,7 +145,8 @@ Dart_Isolate CreateVmServiceIsolateFromKernel(
config.write_service_info_filename,
/*trace_loading=*/false, config.deterministic,
/*enable_service_port_fallback=*/false,
/*wait_for_dds_to_advertise_service=*/false)) {
/*wait_for_dds_to_advertise_service=*/false,
/*serve_observatory*/ true)) {
*error = Utils::StrDup(bin::VmService::GetErrorMessage());
return nullptr;
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/bin/main.cc
Expand Up @@ -555,7 +555,7 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri,
Options::vm_service_auth_disabled(),
Options::vm_write_service_info_filename(), Options::trace_loading(),
Options::deterministic(), Options::enable_service_port_fallback(),
wait_for_dds_to_advertise_service)) {
wait_for_dds_to_advertise_service, !Options::disable_observatory())) {
*error = Utils::StrDup(VmService::GetErrorMessage());
return NULL;
}
Expand Down
5 changes: 5 additions & 0 deletions runtime/bin/main_options.cc
Expand Up @@ -483,6 +483,11 @@ bool Options::ParseArguments(int argc,
// a VM flag as disabling DDS changes how we configure the VM service,
// so we don't need to handle that case here.
skipVmOption = true;
} else if (IsOption(argv[i], "serve-observatory")) {
// This flag is currently set by default in vmservice_io.dart, so we
// ignore it. --no-serve-observatory is a VM flag so we don't need to
// handle that case here.
skipVmOption = true;
}
if (!skipVmOption) {
temp_vm_options.AddArgument(argv[i]);
Expand Down
3 changes: 2 additions & 1 deletion runtime/bin/main_options.h
Expand Up @@ -48,7 +48,8 @@ namespace bin {
V(long_ssl_cert_evaluation, long_ssl_cert_evaluation) \
V(bypass_trusting_system_roots, bypass_trusting_system_roots) \
V(delayed_filewatch_callback, delayed_filewatch_callback) \
V(mark_main_isolate_as_system_isolate, mark_main_isolate_as_system_isolate)
V(mark_main_isolate_as_system_isolate, mark_main_isolate_as_system_isolate) \
V(no_serve_observatory, disable_observatory)

// Boolean flags that have a short form.
#define SHORT_BOOL_OPTIONS_LIST(V) \
Expand Down
3 changes: 2 additions & 1 deletion runtime/bin/run_vm_tests.cc
Expand Up @@ -151,7 +151,8 @@ static Dart_Isolate CreateAndSetupServiceIsolate(const char* script_uri,
/*write_service_info_filename*/ "",
/*trace_loading=*/false, /*deterministic=*/true,
/*enable_service_port_fallback=*/false,
/*wait_for_dds_to_advertise_service*/ false)) {
/*wait_for_dds_to_advertise_service*/ false,
/*serve_observatory*/ true)) {
*error = Utils::StrDup(bin::VmService::GetErrorMessage());
return nullptr;
}
Expand Down
7 changes: 6 additions & 1 deletion runtime/bin/vmservice_impl.cc
Expand Up @@ -119,7 +119,8 @@ bool VmService::Setup(const char* server_ip,
bool trace_loading,
bool deterministic,
bool enable_service_port_fallback,
bool wait_for_dds_to_advertise_service) {
bool wait_for_dds_to_advertise_service,
bool serve_observatory) {
Dart_Isolate isolate = Dart_CurrentIsolate();
ASSERT(isolate != NULL);
SetServerAddress("");
Expand Down Expand Up @@ -196,6 +197,10 @@ bool VmService::Setup(const char* server_ip,
Dart_NewBoolean(wait_for_dds_to_advertise_service));
SHUTDOWN_ON_ERROR(result);

result = Dart_SetField(library, DartUtils::NewString("_serveObservatory"),
serve_observatory ? Dart_True() : Dart_False());
SHUTDOWN_ON_ERROR(result);

// Are we running on Windows?
#if defined(DART_HOST_OS_WINDOWS)
Dart_Handle is_windows = Dart_True();
Expand Down
3 changes: 2 additions & 1 deletion runtime/bin/vmservice_impl.h
Expand Up @@ -22,7 +22,8 @@ class VmService {
bool trace_loading,
bool deterministic,
bool enable_service_port_fallback,
bool wait_for_dds_to_advertise_service);
bool wait_for_dds_to_advertise_service,
bool serve_observatory);

static void SetNativeResolver();

Expand Down
2 changes: 2 additions & 0 deletions sdk/lib/_internal/vm/bin/vmservice_io.dart
Expand Up @@ -39,6 +39,8 @@ StreamSubscription<ProcessSignal>? _signalSubscription;
bool _enableServicePortFallback = false;
@pragma("vm:entry-point")
bool _waitForDdsToAdvertiseService = false;
@pragma("vm:entry-point")
bool _serveObservatory = true;

// HTTP server.
Server? server;
Expand Down
38 changes: 37 additions & 1 deletion sdk/lib/_internal/vm/bin/vmservice_server.dart
Expand Up @@ -369,7 +369,43 @@ class Server {
}
return;
}

if (!_serveObservatory) {
final ddsUri = _service.ddsUri;
if (ddsUri == null) {
request.response.headers.contentType = ContentType.text;
request.response.write('This VM does not have a registered Dart '
'Development Service (DDS) instance and is not currently serving '
'Dart DevTools.');
request.response.close();
return;
}
// We build this path manually rather than manipulating ddsUri directly
// as the resulting path requires an unencoded '#'. The Uri class will
// always encode '#' as '%23' in paths to avoid conflicts with fragments,
// which will result in the redirect failing.
final path = StringBuffer();
// Add authentication code to the path.
if (ddsUri.pathSegments.length > 1) {
path.writeAll([
ddsUri.pathSegments
.sublist(0, ddsUri.pathSegments.length - 1)
.join('/'),
'/',
]);
}
final queryComponent = Uri.encodeQueryComponent(
ddsUri.replace(scheme: 'ws', path: '${path}ws').toString(),
);
path.writeAll([
'devtools/#/',
'?uri=$queryComponent',
]);
final redirectUri = Uri.parse(
'http://${ddsUri.host}:${ddsUri.port}/$path',
);
request.response.redirect(redirectUri);
return;
}
if (assets == null) {
request.response.headers.contentType = ContentType.text;
request.response.write('This VM was built without the Observatory UI.');
Expand Down

0 comments on commit 3602351

Please sign in to comment.