Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ Breaking Change Request ] Remove Observatory from the Dart VM #50233

Open
bkonyi opened this issue Oct 17, 2022 · 23 comments
Open

[ Breaking Change Request ] Remove Observatory from the Dart VM #50233

bkonyi opened this issue Oct 17, 2022 · 23 comments
Labels
area-vm breaking-change-request This tracks requests for feedback on breaking changes triaged Issue has been triaged by sub team

Comments

@bkonyi
Copy link
Contributor

bkonyi commented Oct 17, 2022

The Observatory developer tooling was developed during the early days of the Dart project to allow for developers to debug, dive into CPU and memory performance characteristics, and inspect program structure of Dart programs. This tool was developed and maintained solely by Dart VM engineers without much input from UX designers, making it powerful for Dart SDK developers but difficult to use by the average Dart developer.

With the development of Dart DevTools by a dedicated team, it no longer makes sense to continue work on Observatory with the associated compatibility and maintenance burdens, especially now that Dart DevTools is at near feature parity with Observatory.

Impact:

Pre-Dart 3.0, users wishing to continue using Observatory will need to pass an --enable-observatory flag for Observatory to be served by the VM. For Dart 3.0, this flag will be removed and remaining Observatory users will be required to move to the new Dart DevTools developer tooling.

Dart DevTools is at near feature parity with Observatory and should meet the needs of the vast majority of Dart developers. Any missing functionality in DevTools that's critical to an individual's workflow should be documented in a new issue on the DevTools issue tracker.

Proposal:

  • Pre-Dart 3.0: place Observatory behind --enable-observatory, making Observatory disabled by default by still available.
  • Dart 3.0: remove the --enable-observatory flag and remove Observatory completely from the SDK.

cc @a-siva @kenzieschmoll @mit-mit
@bkonyi bkonyi added the breaking-change-request This tracks requests for feedback on breaking changes label Oct 17, 2022
@lrhn lrhn added the area-vm label Oct 18, 2022
@a-siva
Copy link
Contributor

a-siva commented Oct 18, 2022

//cc @zanderso

@zanderso
Copy link
Member

The --enable-observatory flag should be made available before it is required. This is so that we can make changes to pass the flag without losing access to the Observatory.

@bkonyi
Copy link
Contributor Author

bkonyi commented Oct 19, 2022

The --enable-observatory flag should be made available before it is required. This is so that we can make changes to pass the flag without losing access to the Observatory.

SGTM.

@bkonyi
Copy link
Contributor Author

bkonyi commented Nov 3, 2022

CL adding support for --[no-]serve-observatory is up for review. Default behavior currently is to implicitly set --serve-observatory.

@itsjustkevin
Copy link
Contributor

@vsmenon @Hixie @grouma @zanderso it seems like the comments here have been addressed by @bkonyi . How do we feel about this breaking change request?

@zanderso
Copy link
Member

zanderso commented Nov 8, 2022

I'm not sure the timeline is achievable, but otherwise the plan sgtm. I also added flutter/devtools#4711 to the tracker.

@a-siva
Copy link
Contributor

a-siva commented Nov 9, 2022

lgtm with the caveat that the performance of devtools is acceptable to the engineers who currently use observatory

@vsmenon
Copy link
Member

vsmenon commented Nov 9, 2022

lgtm

@itsjustkevin
Copy link
Contributor

@bkonyi do you find you have enough time to implement this?

@bkonyi
Copy link
Contributor Author

bkonyi commented Nov 9, 2022

The initial work needed to allow for disabling Observatory by default is already up for review. Once that change lands and flutter_tools has the right plumbing for --serve-observatory, we can go ahead and change the default behavior to not serve Observatory.

@itsjustkevin
Copy link
Contributor

Thanks @bkonyi, only thing left to address here is the comment by @a-siva.

@zanderso
Copy link
Member

zanderso commented Nov 9, 2022

@bkonyi --serve-observatory needs to be a flag that the Engine can pass to Dart_SetVMFlags. Is that what your change did?

@bkonyi
Copy link
Contributor Author

bkonyi commented Nov 15, 2022

@bkonyi --serve-observatory needs to be a flag that the Engine can pass to Dart_SetVMFlags. Is that what your change did?

@zanderso I think we'll just need to make a change here in the engine to allow for this option to be set.

@zanderso
Copy link
Member

@bkonyi That sounds plausible to me, but you'd be the expert on where and when the new flag needs to be set in the service isolate code.

@rmacnak-google
Copy link
Contributor

Many gaps in functionality between Observatory and DevTools are filed under this tag

copybara-service bot pushed a commit that referenced this issue Nov 21, 2022
@bkonyi
[ VM Service ] Add support for '--[no-]serve-observatory'
3602351 
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>
@mit-mit mit-mit mentioned this issue Dec 13, 2022
Closed
16 tasks
@a-siva a-siva added triaged Issue has been triaged by sub team and removed vm-triaged labels Dec 20, 2022
bkonyi added a commit to flutter/flutter that referenced this issue Jan 16, 2023
@bkonyi
Remove references to Observatory
96f68e1 
Observatory is being deprecated for Dart 3.0 so it should no longer be
referenced in tooling messaging / flags.

See dart-lang/sdk#50233
@bkonyi bkonyi mentioned this issue Jan 16, 2023
Merged
bkonyi added a commit to flutter/engine that referenced this issue Jan 16, 2023
@bkonyi
Remove references to Observatory
2c12106 
Observatory is being deprecated for Dart 3.0 so it should no longer be referenced in tooling messaging / flags.

See dart-lang/sdk#50233
@bkonyi bkonyi mentioned this issue Jan 16, 2023
Merged
bkonyi added a commit to flutter/engine that referenced this issue Jan 19, 2023
@bkonyi
Remove references to Observatory (#38919)
88e61d8 
Observatory is being deprecated for Dart 3.0 so it should no longer be referenced in tooling messaging / flags.

See dart-lang/sdk#50233
ricardoamador pushed a commit to ricardoamador/engine that referenced this issue Jan 25, 2023
@bkonyi @ricardoamador
Remove references to Observatory (flutter#38919)
9234df1 
Observatory is being deprecated for Dart 3.0 so it should no longer be referenced in tooling messaging / flags.

See dart-lang/sdk#50233
@srawlins
Copy link
Member

Sorry for my ignorance, will we also remove the --observe= flag?

@itsjustkevin
Copy link
Contributor

@bkonyi and @rmacnak-google are we planning on keeping this open until all issues in this tag are resolved?

bkonyi added a commit to flutter/flutter that referenced this issue Feb 13, 2023
@bkonyi
Remove references to Observatory (#118577)
2df140f 
Observatory is being deprecated for Dart 3.0 so it should no longer be
referenced in tooling messaging / flags.

See dart-lang/sdk#50233
@bkonyi
Copy link
Contributor Author

bkonyi commented Mar 9, 2023

@bkonyi and @rmacnak-google are we planning on keeping this open until all issues in this tag are resolved?

I don't know if we'll end up porting everything under that tag as some of the functionality isn't critical / widely used, so we'll need to make a judgement call at some point.

Sorry for my ignorance, will we also remove the --observe= flag?

@srawlins definitely not :-)

@bkonyi
Copy link
Contributor Author

bkonyi commented Mar 9, 2023

CL to disable serving Observatory by default is up for review.

@itsjustkevin
Copy link
Contributor

Thanks @bkonyi.

CC: @XilaiZhang and @CaseyHillers as this may impact flutter rolls.

copybara-service bot pushed a commit that referenced this issue Mar 10, 2023
@bkonyi
[ Observatory ] Disable serving Observatory by default
edead9c 
Observatory can still be enabled by providing `--serve-observatory` or
invoking the `_serveObservatory` private service RPC via web socket or
HTTP.

Related to #50233

TEST=pkg/dartdev/test/commands/run_test

Change-Id: I89b000e69bb31c91a9a5386fed1ee590cdafa58c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287821
Reviewed-by: Michael Thomsen <mit@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
bkonyi added a commit to flutter/flutter that referenced this issue Mar 10, 2023
@bkonyi
Stop serving Observatory by default
4046b2a 
Observatory can still be enabled by providing `--serve-observatory` or
invoking the `_serveObservatory` private service RPC via web socket or
HTTP.

Related to dart-lang/sdk#50233
@bkonyi bkonyi mentioned this issue Mar 10, 2023
Merged
copybara-service bot pushed a commit that referenced this issue Mar 14, 2023
@a-siva
Revert "[ Observatory ] Disable serving Observatory by default"
5a8ddc0 
This reverts commit edead9c.

Reason for revert: We have some flutter framework tests that are breaking, "flutter test should respect --serve-observatory". This will apparently be fixed when flutter/flutter#122419 lands.

Original change's description:
> [ Observatory ] Disable serving Observatory by default
>
> Observatory can still be enabled by providing `--serve-observatory` or
> invoking the `_serveObservatory` private service RPC via web socket or
> HTTP.
>
> Related to #50233
>
> TEST=pkg/dartdev/test/commands/run_test
>
> Change-Id: I89b000e69bb31c91a9a5386fed1ee590cdafa58c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287821
> Reviewed-by: Michael Thomsen <mit@google.com>
> Commit-Queue: Ben Konyi <bkonyi@google.com>
> Reviewed-by: Siva Annamalai <asiva@google.com>

Change-Id: I994c86cbca9d0eb25e1f9adddeede94c227acad9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/288601
Commit-Queue: Siva Annamalai <asiva@google.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Zach Anderson <zra@google.com>
bkonyi added a commit to flutter/flutter that referenced this issue Mar 23, 2023
@bkonyi
Stop serving Observatory by default (#122419)
5a36bdd 
Observatory can still be enabled by providing `--serve-observatory` or
invoking the `_serveObservatory` private service RPC via web socket or
HTTP.

Related to dart-lang/sdk#50233
@mit-mit
Copy link
Member

mit-mit commented Apr 17, 2023

@bkonyi looks like this landed, but got reverted?

@bkonyi
Copy link
Contributor Author

bkonyi commented Apr 17, 2023

@mit-mit that's correct. We were having issues getting this landed among all the other breaking changes and it was decided this was lower priority to get into 3.0. We're going to target the next stable release instead.

@mit-mit
Copy link
Member

mit-mit commented Apr 17, 2023

OK, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-vm breaking-change-request This tracks requests for feedback on breaking changes triaged Issue has been triaged by sub team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
9 participants
@srawlins @vsmenon @lrhn @zanderso @rmacnak-google @a-siva @itsjustkevin @mit-mit @bkonyi