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

Sum stats instead of last #172

Merged
merged 1 commit into from
Nov 23, 2022
Merged

Conversation

Meldiron
Copy link
Contributor

What does this PR do?

Instead of showing last 1H/1D stat in overview, we sum all results.

Test Plan

  • Manual QA

Related PRs and Issues

x

Have you read the Contributing Guidelines on issues?

Yes

@vercel
Copy link

vercel bot commented Nov 21, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
console ✅ Ready (Inspect) Visit Preview Nov 21, 2022 at 9:59AM (UTC)

@Meldiron
Copy link
Contributor Author

Before:
CleanShot 2022-11-21 at 11 03 34

After:

CleanShot 2022-11-21 at 11 03 25

@@ -89,14 +89,14 @@

<div class="grid-item-1-end-start">
<div class="heading-level-4">
{format(last($usage.documents).value)}
{format(total($usage.documents))}
Copy link
Contributor

Choose a reason for hiding this comment

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

the one for documents should be fine 👍🏻 they dont need any change

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 only executions are different

Copy link
Member

Choose a reason for hiding this comment

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

All these should be summed, as all these are now using time-series data, before we got sum from database and data already had the sum, now it's not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that after usage refactor the is no more aggregation and we ONLY have time series data. So we should be summing everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, not the best solution since we don't have real total, just total in time period. But its a patch until usage container can properly sum.

Copy link
Contributor

@TorstenDittmann TorstenDittmann left a comment

Choose a reason for hiding this comment

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

👍🏻

@christyjacob4 christyjacob4 changed the base branch from main to db-pools-support November 23, 2022 18:24
@christyjacob4 christyjacob4 merged commit 759f3e2 into db-pools-support Nov 23, 2022
@ArmanNik ArmanNik deleted the fix-overview-stats branch August 30, 2023 16:24
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.

4 participants