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

fix: backward compatibility for qtree template #1679

Merged
merged 7 commits into from
Feb 7, 2023
Merged

fix: backward compatibility for qtree template #1679

merged 7 commits into from
Feb 7, 2023

Conversation

Hardikl
Copy link
Contributor

@Hardikl Hardikl commented Feb 1, 2023

In 22.05,

qtree_labels:
image

qtree_disk_llimit:
image

No quota_xxx metrics exist.

In 23.02, without any changes,
qtree_labels:
image

qtree_disk_limit:
image

quota_disk_limit:
image

In 23.02, with changes with qtree.yaml,
qtree_labels:
image

qtree_disk_limit:
image

quota_disk_limit:
image

In 23.02, when changed from qtree.yaml to qtree.22.05.yaml in default.yaml
qtree_labels:
image

qtree_disk_limit:
image

quota_disk_limit:
image

Points:

  • labels of qtree_labels would be same in both the template.
  • no of records of qtree_labels would be differ in both template --> reason: latest template excludes empty qtree names.
  • no of records of quota metrics would be differ in both template --> reason: latest template excludes empty qtree names.

Current Issue:

  • latest template would expect all type of quota when no quota-type filter applied in template, whereas old template would expect only tree quotas.

@Hardikl
Copy link
Contributor Author

Hardikl commented Feb 1, 2023

Added minor change in older template to fetch only tree quotas as Harvest 22.05 only gives tree quotas.

Reason: We have only one Qtree plugin which also contains all the latest changes. It would be easier to get same metric behaviour from the template change than plugin changes.

Metrics when changed to qtree.22.05.yaml in default.yaml.
qtree_labels:
image

qtree_disk_limit:
image

quota_disk_limit:
image

Points:

  • labels of qtree_labels would be same in both the template.
  • 3 extra labels[index, type, unit] would be present in qtree_xxx metric in 22.05 template
  • no of records of qtree_labels, qtree_xxx would be same in both the template.
  • qtree.22.05.yaml gives same behaviour as Harevst 22.05
  • qtree.yaml gives current behaviour

@Hardikl Hardikl linked an issue Feb 1, 2023 that may be closed by this pull request
@Hardikl
Copy link
Contributor Author

Hardikl commented Feb 3, 2023

Case 1: Same behaviour as Harvest 22.05
with hidden key-val pair
qtree_labels:
image

qtree_disk_limit:
image

quota_xxxx metrics not exist.

Case 2: Current Harvest behaviour
without hidden key-val pair:
qtree_labels:
image

qtree_disk_limit:
image

quota_disk_limit:
image

@Hardikl
Copy link
Contributor Author

Hardikl commented Feb 3, 2023

Change update in PR:
with hidden flag --> both qtree and quota metrics will be visible with new labels as well as older labels.
without hidden flag --> both qtree and quota metrics will be visible with new labels only.


// apply all instance keys, instance labels from parent (qtree.yaml) to all quota metrics
//parent instancekeys would be added in plugin metrics
for _, parentKeys := range my.ParentParams.GetChildS("export_options").GetChildS("instance_keys").GetAllChildContentS() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

are we sure that each of these nodes will be present, otherwise we will panic on nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's export section from qtree template, it should always be there to define which keys/labels would be exported for qtree.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

cmd/collectors/zapi/plugins/qtree/qtree.go Outdated Show resolved Hide resolved
@Hardikl Hardikl merged commit 817eef0 into main Feb 7, 2023
@Hardikl Hardikl deleted the qtree_change branch February 7, 2023 08:21
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.

Restore qtree disk metrics
2 participants