-
Notifications
You must be signed in to change notification settings - Fork 304
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-33167 Add unique ID to each metric #19394
Conversation
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-33167 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.
@kenrowland the change seems fine, but it's not clear what practical advantage is gained.
Also, are the ids simple sequential ints across the entire HPCC cluster, or is this per process?
@rpastrana The Jira covers an intended use. The unique ID is per process. Technically per loaded jlib. |
Thanks for that @kenrowland, I asked because nothing seems to be consuming the id. So the intended use per the jira is to "avoid the repeated reconstruction of the fully qualified metric names" but this change only adds the unique ID, and there's no linked jira to make use of the unique ID. I understand you're making a concerted effort to keep your PRs at manageable size, but I'd consider either including the uid consumer logic here (it would be within scope of jira) or create the corresponding jira and linking it. If the consumer of the uid is expected to be outside of hpcc, please update the jira description as well. |
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.
per offline convo, consider changing the id member to unsigned, and add link to jira proposed to utilize this feature
Added unique ID to each metric when constructed Signed-Off-By: Kenneth Rowland kenneth.rowland@lexisnexisrisk.com
05acba2
to
d8b1d29
Compare
HPCC-33204 added to make use of this change. |
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.
@kenrowland seems fine
@ghalliday Please merge |
I am also not sure of the benefit of this change, but I will merge it. |
Jirabot Action Result: |
Added unique ID to each metric when constructed
Signed-Off-By: Kenneth Rowland kenneth.rowland@lexisnexisrisk.com
Type of change:
Checklist:
Smoketest:
Testing: