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.

Sign up for GitHub

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

Adding EventMeter Trigger #3812

Merged
merged 23 commits into from
Mar 15, 2023
Merged

Adding EventMeter Trigger #3812

merged 23 commits into from
Mar 15, 2023

Conversation

kkeirstead
Copy link
Member

@kkeirstead kkeirstead commented Feb 28, 2023 •
edited
Loading

Summary

This PR adds a trigger for SystemDiagnosticsMetrics. The options are similar to the EventCounter trigger, with two differences:

  • Instead of CounterName, InstrumentName is used (there had previously been some discussion about unifying these names to reduce mental burden for users - I'm leaving it as is for now, but think it's worth discussing on this PR).
  • An additional property named HistogramPercentile that allows users to set which percentile they want to compare against for the GreaterThan/LessThan value. For now, this value would be either 50/95/99.

Docs and tests have been updated as part of this PR.

There is also a PR in the diagnostics repo for this feature.

Release Notes Entry

Added EventMeter Trigger for Collection Rules.

@kkeirstead kkeirstead requested a review from a team as a code owner February 28, 2023 17:09
@kkeirstead kkeirstead mentioned this pull request Feb 28, 2023
Merged
wiktork
wiktork reviewed Mar 6, 2023
View reviewed changes
Copy link
Member

@wiktork wiktork left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, minus the additional properties for Provider/CounterName, which I think we should remove.

wiktork
wiktork previously approved these changes Mar 10, 2023
View reviewed changes
@kkeirstead kkeirstead added the update-release-notes Pull requests that should be mentioned in the release notes label Mar 15, 2023
@kkeirstead kkeirstead changed the title Adding SystemDiagnosticsMetrics Trigger Adding EventMeter Trigger Mar 15, 2023
@kkeirstead kkeirstead merged commit 2e9e71a into dotnet:main Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@jander-msft jander-msft jander-msft approved these changes

@wiktork wiktork wiktork left review comments

Assignees
No one assigned
Labels
update-release-notes Pull requests that should be mentioned in the release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kkeirstead @wiktork @jander-msft