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.
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
[Instrumentation.EventCounters] Stabilize and Enable Testing #620
[Instrumentation.EventCounters] Stabilize and Enable Testing #620
Changes from 30 commits
57986a6
51d9cce
59e4faa
2e2af13
8fbb353
e13c37f
3eb1061
97ed727
732bc59
a1566d1
1f5b50e
60938d3
adca7c5
37414ca
1a0c496
906b84c
d5c7fdf
8d0a30e
c8af401
99097fc
78d981d
f73858b
b44f952
1ef006e
12cbb30
ecaeea4
9398617
e558609
72e52d1
54d8b2c
6daa63d
553ae87
556b65e
71560ed
65521ee
a6bfef2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method feels like it should belong to
EventCountersMetrics
class.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then the
eventSouceNames
HashSet would also move and then this class would be empty. I'll keep here but I kinda see your pointThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EventSourceNames
is also another good candidate to belong toEventCountersMetrics
class. The options class won't be empty. It will still have the "options" 😀There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, it falls more into the responsibility of the
EventCountersMetrics
class to determine what it should listen rather than an options class.