[ Breaking Change Request ] Remove support for Metrics from dart:developer

[bkonyi]

Since the early days of the VM service protocol, the Dart VM has provided APIs for visualizing instantaneous metrics in graphs within Observatory through dart:developer, exposed through the Metrics interface.

Rationale:

This feature has been broken for quite some time in Observatory as we lost the ability to display charts and graphs as a consequence of the Dart 2.0 and null-safety migrations. DevTools also does not currently support displaying metrics, so it seems like a good time to trim an unused API.

Impact:

Deprecating and removing the Metrics APIs should have little to no impact as they haven't worked in quite some time. There's no obvious uses in g3, and there have been no issues filed regarding the broken Observatory functionality in the years since the breakage occurred. There's also no breakages to the VM service protocol as a result of this change, as the existing metrics RPCs are private and were never publicly exposed.

Proposal:
Deprecate Metrics, Metric, Counter, and Gauge, and schedule them for removal in Dart 3.0.

cc @a-siva @kenzieschmoll

[sigmundch]

@bkonyi - I fear this will indirectly break the support we just added in #49062 to expose internal metrics like memory usage and capacity numbers, correct?

Will we offer any alternative to that other than the current logic we use in dart2js? (where we launch a the service protocol and use a vm service client within the process to read this data).

[bkonyi]

@bkonyi - I fear this will indirectly break the support we just added in #49062 to expose internal metrics like memory usage and capacity numbers, correct?

Will we offer any alternative to that other than the current logic we use in dart2js? (where we launch a the service protocol and use a vm service client within the process to read this data).

No, this shouldn't impact internal memory statistics accessed via the service protocol's private metric RPC. If I recall correctly, we ended up discovering that the native metrics weren't being exposed properly by #49062 anyway since they are defined in native code, not in the Metrics map in dart:developer.

[sigmundch]

Thanks for clarifying - I thought they had been exposed and that we just didn't get around to convert our logic to use it.

[a-siva]

//cc @zanderso

[itsjustkevin]

@sigmundch do you feel comfortable with this now that @bkonyi has addressed your concerns?

[sigmundch]

Yes, thanks for checking. sgtm

[itsjustkevin]

Thanks for the response @sigmundch, with that, @vsmenon, @grouma, @Hixie can you take a look?

[vsmenon]

@a-siva @kenzieschmoll - do you both lgtm?

[a-siva]

lgtm

[Hixie]

fine by me since it's already broken anyway. did anyone file a bug asking for it to be fixed?

[kenzieschmoll]

DevTools doesn't use Metrics for anything. LGTM.

[bkonyi]

@itsjustkevin is it safe to assume this can go forward? :-)

[bkonyi]

Deprecation CL is ready to land once this is approved.

[bkonyi]

fine by me since it's already broken anyway. did anyone file a bug asking for it to be fixed?

Nope, I don't think anyone really even noticed it was broken in the first place.

[itsjustkevin]

@grouma opinions on this breaking change?

[grouma]

I'm not aware of any usage from the ACX and or debugging side.

[itsjustkevin]

Thank you @grouma, with that, I think we can approve this breaking change @bkonyi.

[itsjustkevin]

@bkonyi I see this was merged in November and would like to close this, but don't see a changelog entry for this. Am I just missing it?

[bkonyi]

Hm, thought I had included a changelog entry, but looks like I haven't. I'll add one.

[itsjustkevin]

Thanks @bkonyi! Feel free to close the issue when the changelog entry is added.

[itsjustkevin]

Changelog updated in https://dart-review.googlesource.com/c/sdk/+/278531. Closing this issue.