search

dotnet / diagnostics

This repository contains the source code for various .NET Core runtime diagnostic tools and documents.
MIT License
1.18k stars 354 forks source link

`dotnet counters list` defaults to out of support `--runtime-version 3.1` #3622

Closed halter73 closed 1 year ago

halter73 commented 1 year ago

It doesn't look like dotnet counters list has gotten any major updates since https://github.com/dotnet/diagnostics/pull/1369, but we've added a bunch of well-known counters. It still defaults to --runtime-version 3.1 which is no longer supported, and it hasn't been updated to include well-known counters since .NET 5 like the .NET 6 and 7 networking counters added to our docs with https://github.com/dotnet/docs/pull/31804 by @MihaZupan.

We should probably add support for --runtime-version 6.0 and 7.0 and update the default 6.0. And we need to think about how we prevent ourselves from defaulting to out of support versions in the future because this can lead to confusion as demonstrated in https://github.com/dotnet/aspnetcore/issues/46135#issuecomment-1401836632 since this made the Kestrel counters hard to find.

mikem8361 commented 1 year ago

@dotnet/dotnet-diag we may want to fix this for our upcoming tools release.

MihaZupan commented 1 year ago

I thought I added those to dotnet-counters as well in #3470 -- does something else need to happen to light that up?

halter73 commented 1 year ago

I thought I added those to dotnet-counters as well in #3470 -- does something else need to happen to light that up?

Nice! I think we'll also have to update the --runtime-version default from 3.1 to 6.0 and add to s_SupportedRuntimeVersions, but it looks like we're closer than I thought.

https://github.com/dotnet/diagnostics/blob/615b21dd10832c0faaaa93d5f177e491ce9b3a09/src/Tools/dotnet-counters/Program.cs#L165

https://github.com/dotnet/diagnostics/blob/615b21dd10832c0faaaa93d5f177e491ce9b3a09/src/Tools/dotnet-counters/Program.cs#L210

I'm guessing we'll need to extend SupportedVersions=new[] { net30, net31, net50, net60 } and similar in KnownData.cs to include net70 as well. And it'd be nice to think about how we make sure we don't let the default --runtime-version to become so out of date again. Can we add a test that verifies the default is the latest LTS runtime release?

tommcdon commented 1 year ago

@davmason @dramos020

noahfalk commented 1 year ago

I don't want to derail any short term fix like changing a default version, but for the longer term I think we should consider what experience we want overall. I'm not convinced indefinitely updating a hard-coded list is the best place to be. I opened https://github.com/dotnet/diagnostics/issues/3626 to hopefully avoid creating too much noise here.