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

Only capture metrics for supported ContainerImages #21829

Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Apr 21, 2022

The OpenShift provider collects a mixture of ContainerImages some of which support metrics capture and some which do not. We have an issue with ContainerImage class names (#21828) but even with that fixed fundamentally we are trying to capture metrics from ContainerImages which do not support it.

Fixes #21825

@agrare
Copy link
Member Author

agrare commented Apr 21, 2022

>> ems.container_images.count
   (0.7ms)  SELECT COUNT(*) FROM "container_images" WHERE "container_images"."ems_id" = $1 AND "container_images"."deleted_on" IS NULL  [["ems_id", 2]]
=> 363
>> ems.container_images.supporting(:capture).count
  ContainerImage Load (1.5ms)  SELECT "container_images".* FROM "container_images" WHERE "container_images"."ems_id" = $1 AND "container_images"."deleted_on" IS NULL AND "container_images"."type" = $2  [["ems_id", 2], ["type", "ManageIQ::Providers::Openshift::ContainerManager::ContainerImage"]]
  ContainerImage Inst Including Associations (119.1ms - 306rows)
=> 306

@agrare
Copy link
Member Author

agrare commented Apr 21, 2022

We have this handy supports feature already https://github.com/ManageIQ/manageiq/blob/master/app/models/metric/ci_mixin.rb#L23-L28 and the InfraManager::MetricsCapture class already uses it for hosts and vms, wondering if we should use this for all of the target types

@agrare agrare requested a review from Fryguy April 21, 2022 19:37
@agrare agrare force-pushed the only_capture_metrics_supported_container_images branch from c211c64 to cc482fa Compare April 22, 2022 13:29
@miq-bot
Copy link
Member

miq-bot commented Apr 22, 2022

Checked commit agrare@cc482fa with ruby 2.6.9, rubocop 1.19.1, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. ⭐

@Fryguy Fryguy merged commit 32750cf into ManageIQ:master Apr 26, 2022
@Fryguy Fryguy self-assigned this Apr 26, 2022
@agrare agrare deleted the only_capture_metrics_supported_container_images branch April 26, 2022 15:27
@Fryguy
Copy link
Member

Fryguy commented May 9, 2022

Backported to najdorf in commit 5d665cd.

commit 5d665cdd06b9aa86062be67168434311f882715d
Author: Jason Frey <[email protected]>
Date:   Tue Apr 26 11:13:15 2022 -0400

    Merge pull request #21829 from agrare/only_capture_metrics_supported_container_images
    
    Only capture metrics for supported ContainerImages
    
    (cherry picked from commit 32750cf7e828ebb14f235c8470a2c8653db397bd)

Fryguy added a commit that referenced this pull request May 9, 2022
…container_images

Only capture metrics for supported ContainerImages

(cherry picked from commit 32750cf)
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.

Error Metrics Openshift
3 participants