-
Notifications
You must be signed in to change notification settings - Fork 78
app/vmui: feature: add selectable bars count for hits/stats charts #958
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
base: master
Are you sure you want to change the base?
Conversation
…ictoriaMetrics#956) - add bars count dropdown with predefined steps (12..1240, default 96) - persist bars count across URL/search params and reuse for Query/Overview flows - pass bars count through fetch, time bucketing, and uPlot rendering for consistent bar sizing - allow SERVER_URL override via localStorage for remote backend testing Fixes VictoriaMetrics#956
|
@withlin , thank you for the pull request! @Loori-R , @arturminchukov , could you review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good. I left a couple of comments to sync this PR with the new bar rendering logic from PR #988 and avoid conflicts. Also, please add an entry to docs/victorialogs/CHANGELOG.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sync with PR #988 (bar rendering fix), this file should not be changed. paths no longer depends on bars count.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sync with PR #988: app/vmui/packages/vmui/src/utils/uplot/paths.ts will be removed, so please avoid making changes here.
| }; | ||
|
|
||
| export const getHitsTimeParams = (period: TimeParams) => { | ||
| export const getHitsTimeParams = (period: TimeParams, barsCount = LOGS_DEFAULT_BAR_COUNT) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid conflicts with PR #988, could you align this with the implementation from
VictoriaLogs/app/vmui/packages/vmui/src/utils/logs.ts
Lines 12 to 22 in 08d08b9
| export const getHitsTimeParams = (period: TimeParams) => { | |
| const start = dayjs(period.start * 1000); | |
| const end = dayjs(period.end * 1000); | |
| const totalMs = Math.max(1, end.diff(start, "ms")); | |
| const bars = Math.min(LOGS_BARS_VIEW, totalMs); | |
| const step = Math.max(1, Math.ceil(totalMs / bars)); | |
| const offset = ((start.valueOf() % step) + step) % step; | |
| return { start, end, step, offset }; | |
| }; |
Fixes #956
default.mov