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.

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

feat: Harvest should collect QOS policy groups #1831

Merged
merged 3 commits into from
Mar 21, 2023
Merged

Conversation

cgrinds
Copy link
Collaborator

@cgrinds cgrinds commented Mar 20, 2023

No description provided.

Hardikl
Hardikl previously approved these changes Mar 21, 2023
Hardikl
Hardikl previously approved these changes Mar 21, 2023
Comment on lines +48 to +51
instance.SetLabel("min_throughput_iops", min.IOPS)
instance.SetLabel("max_throughput_iops", max.IOPS)
instance.SetLabel("min_throughput_mbps", min.Mbps)
instance.SetLabel("max_throughput_mbps", max.Mbps)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the label be absent/empty instead of having 0 values. For example if qos policy is 19.07MB/s-4.55TB/s then both min_throughput_iops and max_throughput_iops should not be published for that qos policy.

@@ -14,6 +14,7 @@ objects:
# NetPort: netPort.yaml
Node: node.yaml
NtpServer: ntpserver.yaml
QosPolicy: qos_policy.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

should be qos_policy_fixed.yaml

Copy link
Contributor

Choose a reason for hiding this comment

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

I think name qos_policy is better. We can differentiate between adapative and non-adaptive with a label in exported metrics? Avoids different names?

@cgrinds cgrinds merged commit e64c43f into main Mar 21, 2023
@cgrinds cgrinds deleted the cbg-qos-policy-1426 branch March 21, 2023 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add information about the resources defined by every policy-group (QoS)
3 participants