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

[bug] PR Time to Merge should use average, not greatest #85

Open
jberkus opened this issue Nov 15, 2024 · 5 comments
Open

[bug] PR Time to Merge should use average, not greatest #85

jberkus opened this issue Nov 15, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@jberkus
Copy link

jberkus commented Nov 15, 2024

When looking at larger intervals of time, Kubernetes PR-Time-To-Merge uses this calculation:

greatest(percentile_disc(0.5) within group (order by open_to_lgtm asc), 0) as m_o2l_a,

This is wrong; it leads to having a greater value the larger your time interval is, which is deceptive. For aggregating a median, there's two reasonable possibilities: a median of medians, or an average of medians. In practice, those two values are rarely that divergent, and an average is faster to calculate.

Therefore it should be:

avg(percentile_disc(0.5) within group (order by open_to_lgtm asc), 0) as m_o2l_a,

This mistake exists across all cacluations in this view.

Assigning to myself, will submit a PR later.

/assign

@jberkus jberkus added the bug Something isn't working label Nov 15, 2024
@lukaszgryglicki
Copy link
Member

I'm OK with this change, just cc @caniszczyk - this is now calculated according to some specs that were given initially long time ago...

@caniszczyk
Copy link

caniszczyk commented Nov 16, 2024 via email

@lukaszgryglicki
Copy link
Member

lukaszgryglicki commented Nov 16, 2024

Sp @jberkus pls create a PR and ping me, once merged I'll regenerate all dashboards - the question is:

  • do we need this change in K8s only, or across all projects? If all then please also change metric in "metrics/all/xyz.sql" in addition to "metrics/kubernetes/xyz.sql".

@jberkus
Copy link
Author

jberkus commented Nov 16, 2024

I haven't checked yet whether this is all projects.

@lukaszgryglicki
Copy link
Member

I mean metric is the same in all projects, my question was: do you want to change in all projects or just in K8s?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants