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

[BURNDOWN-234] filter to correct KSM implementation #2134

Merged
merged 2 commits into from Aug 31, 2023

Conversation

ameijer
Copy link
Contributor

@ameijer ameijer commented Aug 31, 2023

What does this PR change?

  • filters out zero values for kube_container_status_running readings. This is due to kubecosts' exported KSM behaving differently than upstream KSM. The following screenshot illustrates the problem:
    image

A query for a certain pod that ran for 1 hour returns 2 results: 1 set of values is emitted from kubecosts' implementation of the KSM container status running metric, the other results is from another KSM that came bundled with prometheus. As you can see, the light blue line simply stops returning results when the pod exists, however, the third party KSM keeps emitting the metric but sets it to 0 when the pod exits. Our prom query was just querying for all values and taking the date of the last result, which from the other KSM installation, was the current time even if the value was 0. Our logic then thought the pod was still alive By adding the non zero filter, we stop emitting 0 metrics and pull the upstream KSM implementation query results more into line with what we typically emit.

Does this PR relate to any other PRs?

  • None

How will this PR impact users?

  • should make pod run times more reasonable

Does this PR address any GitHub or Zendesk issues?

How was this PR tested?

  • ensured that after this change was made, prom results returned start/end times matching the time the pod was known to be alive

Does this PR require changes to documentation?

  • no

Have you labeled this PR and its corresponding Issue as "next release" if it should be part of the next OpenCost release? If not, why not?

Signed-off-by: Alex Meijer <ameijer@kubecost.com>
Signed-off-by: Alex Meijer <ameijer@kubecost.com>
@nikovacevic nikovacevic merged commit bca6e01 into opencost:develop Aug 31, 2023
3 checks passed
@ameijer ameijer mentioned this pull request Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants