-
Notifications
You must be signed in to change notification settings - Fork 309
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
HPCC-33338 Convert statically allocated metrics to dynamic allocation #19577
base: master
Are you sure you want to change the base?
Conversation
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-33338 Jirabot Action Result: |
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.
PR Overview
This PR converts statically allocated metrics to dynamic allocation and updates the documentation on metric creation methods. Key changes include:
- Revised step-by-step guidance for creating and registering metrics.
- Introduction of different initialization methods including module initialization and construction.
- Updates to code examples to reflect dynamic metric registration.
Reviewed Changes
File | Description |
---|---|
devdoc/Metrics.md | Updated documentation and code examples for metric creation and registration |
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
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.
Looks good from my POV (I only reviewed the MD file)
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.
Seems fine. It might be helpful to describe the problems a developer would encounter and would force them to chose the 3rd option.
will be reported. Essentially, the metric is a black hole. | ||
#### Static Creation | ||
Static creation should be a last resort. If the above methods do not solve the problem of metric | ||
allocation and registering, then static creation is the next best choice. The primary reason to |
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.
What are the symptoms of "problem of metric allocation and registering" that would lead a developer to chose this option?
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.
Added a couple of bullet points detailing general reasons why registering can fail.
Removed statically allocated metrics in favor of using MODULE_INIT Signed-Off-By: Kenneth Rowland [email protected]
@ghalliday Ready for merging unless you'd like Jake to review the metric changes in the Dali source files. |
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.
Looks ok afaics.
@ghalliday Please merge |
Removed statically allocated metrics in favor of using MODULE_INIT
Signed-Off-By: Kenneth Rowland [email protected]
Type of change:
Checklist:
Smoketest:
Testing: