dotnet-counters: Support EventCounter prefix + remove known data #4871
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- dotnet-counters now supports a prefix 'EventCounters\' in front of provider name to force the tool to show EventCounter data when both a Meter and an EventSource have the same name. Without the prefix the Meter data would be shown by default. This is intended to help developers who are migrating between the two and want to see the old data. - dotnet-counters list command no longer shows a hard-coded list of provider and counter names which was already out-of-date. Instead the command now directs users to the online docs. This should be less work for us and more accurate information for users. - dotnet-counters console renderer now sorts all providers alphabetically when displaying them. Previously providers that were in the known provider list had special treatment and got sorted first. Several console exporter tests had to be updated because of the sort order change. - JSON and CSV monitoring tests are re-enabled and use the new EventCounters feature to keep their behavior stable across runtimes given that 9.0 has a System.Runtime Meter and earlier runtimes do not. We already have other tests that handle monitoring Meters and their are tests in the runtime repo for the System.Runtime Meter data. - CounterSet was deleted and ConfigureCounters() was refactored to use List<EventPipeCounterGroup> instead. CounterSet and EventPipeCounterGroup were already similar abstractions so there wasn't much value in maintaining both of them. EventPipeCounterGroup also supports marking a provider as EventCounter only which was needed for EventCounter prefix support. The counter list parsing tests were updated to validate the same results using the EventPipeCounterGroup API instead. -
mdh1418
reviewed
Aug 19, 2024
|
Are there any tests that now need to be re-enabled (and related issue closed)? |
This PR does re-enable a couple tests as part of the change. I'm not aware of others that needed to be reenabled. |
Co-authored-by: Mitchell Hwang <16830051+mdh1418@users.noreply.github.com>
|
@mdh1418 - can you re-approve? Humorously having GH apply your change suggestion invalidates your previous approval :) |
mdh1418
approved these changes
Aug 20, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dotnet-counters now supports a prefix 'EventCounters\' in front of provider name to force the tool to show EventCounter data when both a Meter and an EventSource have the same name. Without the prefix the Meter data would be shown by default. This is intended to help developers who are migrating between the two and want to see the old data.
dotnet-counters list command no longer shows a hard-coded list of provider and counter names which was already out-of-date. Instead the command now directs users to the online docs. This should be less work for us and more accurate information for users.
dotnet-counters console renderer now sorts all providers alphabetically when displaying them. Previously providers that were in the known provider list had special treatment and got sorted first. Several console exporter tests had to be updated because of the sort order change.
JSON and CSV monitoring tests are re-enabled and use the new EventCounters feature to keep their behavior stable across runtimes given that 9.0 has a System.Runtime Meter and earlier runtimes do not. We already have other tests that handle monitoring Meters and there are tests in the runtime repo for the System.Runtime Meter data.
CounterSet was deleted and ConfigureCounters() was refactored to use List<EventPipeCounterGroup> instead. CounterSet and EventPipeCounterGroup were already similar abstractions so there wasn't much value in maintaining both of them. EventPipeCounterGroup also supports marking a provider as EventCounter only which was needed for EventCounter prefix support. The counter list parsing tests were updated to validate the same results using the EventPipeCounterGroup API instead.
@dotnet/dotnet-diag - please take a look :)
@mikem8361 - this PR should fix the test failures you were seeing