fix: charts and logs data count by ogzhanolguncu · Pull Request #3187 · unkeyed/unkey

ogzhanolguncu · 2025-04-24T10:31:07Z

What does this PR do?

This will prevent charts and logs showing the different data. Previously, logs and charts were using different intervals for the default fetch. Reason was to show more data on the chart, but that gives the wrong impression to users.

Fixes # (issue)

Chart and log counts will add up.

Type of change

Bug fix (non-breaking change which fixes an issue)
Chore (refactoring code, technical debt, workflow improvements)
Enhancement (small improvements)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How should this be tested?

Use your seed to generate 1000 data for the metric you wanna check(ratelimits, apis, logs) and check default chart and log count if they add up. It's working.

Checklist

Required

Appreciated

If a UI change was made: Added a screen recording or screenshots to this PR
Updated the Unkey Docs if changes were necessary

BEFORE

AFTER

Active keys counts are already showing min-max for selected interval. It shouldn't be 0-5.

Summary by CodeRabbit

Refactor
- Standardized the time window for fetching timeseries data across dashboard and logs charts to use a consistent historical data window.
Bug Fixes
- Enhanced accuracy of timeseries data retrieval by updating start time calculations to reflect the new historical data window.

This will prevent charts and logs showing the different data. Previously, logs and charts were using different intervals for the default fetch. Reason was to show more data on the chart, but that gives the wrong impression to users.

changeset-bot · 2025-04-24T10:31:12Z

⚠️ No Changeset found

Latest commit: 5bc1ab3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2025-04-24T10:31:12Z

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

Name	Status	Preview	Comments	Updated (UTC)
dashboard	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 24, 2025 10:54am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
engineering	⬜️ Ignored (Inspect)	Visit Preview		Apr 24, 2025 10:54am

coderabbitai · 2025-04-24T10:31:15Z

📝 Walkthrough

Walkthrough

The changes update the way the start time for fetching timeseries data is calculated across several hooks in the dashboard application. Specifically, the constant used for defining the historical data window is switched from TIMESERIES_DATA_WINDOW to HISTORICAL_DATA_WINDOW, and the calculation for the startTime parameter is adjusted accordingly. Additionally, one hook replaces a locally memoized current timestamp with a timestamp from a new useQueryTime hook for query parameter calculation. The constant TIMESERIES_DATA_WINDOW is removed from the constants file. No other logic, control flow, or exported/public entities are modified.

Changes

Files (grouped)	Change Summary
`.../apis/[apiId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts` `.../apis/[apiId]/_overview/components/charts/line-chart/hooks/use-fetch-timeseries.ts` `.../logs/components/charts/hooks/use-fetch-timeseries.ts` `.../ratelimits/[namespaceId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts` `.../ratelimits/[namespaceId]/logs/components/charts/hooks/use-fetch-timeseries.ts` `.../apis/_components/hooks/use-query-timeseries.ts`	Replaced the import of `TIMESERIES_DATA_WINDOW` with `HISTORICAL_DATA_WINDOW` and updated the calculation of the `startTime` parameter to use the new constant. No other logic or control flow was changed.
`.../ratelimits/[namespaceId]/_overview/components/charts/line-chart/hooks/use-fetch-timeseries.ts`	Updated hook to use timestamp from `useQueryTime` instead of local `Date.now()`. Replaced `TIMESERIES_DATA_WINDOW` with `HISTORICAL_DATA_WINDOW` for `startTime` and `endTime` query parameters. Adjusted dependency array accordingly. No other logic changed.
`apps/dashboard/components/logs/constants.ts`	Removed exported constant `TIMESERIES_DATA_WINDOW`. The constant `HISTORICAL_DATA_WINDOW` remains unchanged.
`.../ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.test.tsx`	Updated tests to mock and integrate `QueryTimeProvider` and `useQueryTime` hook. Wrapped hook rendering with `QueryTimeProvider` and adjusted expected console warnings count.

Possibly related PRs

refactor: RQ hook for ratelimits logs #3147: Modifies hooks related to fetching timeseries data by replacing local timestamps with a shared timestamp from the useQueryTime provider and updating the time window constant from TIMESERIES_DATA_WINDOW to HISTORICAL_DATA_WINDOW, indicating a direct connection in query time management across ratelimits logs and overview components.

Suggested reviewers

perkinsjr
mcstepp
chronark

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ef6fe2b and 5bc1ab3.

📒 Files selected for processing (1)

apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.test.tsx (3 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.test.tsx (1)

apps/dashboard/providers/query-time-provider.tsx (1)

QueryTimeProvider (55-72)

⏰ Context from checks skipped due to timeout of 90000ms (6)

GitHub Check: Test Packages / Test ./internal/clickhouse
GitHub Check: Test Packages / Test ./packages/hono
GitHub Check: Test Packages / Test ./packages/api
GitHub Check: Test Packages / Test ./packages/cache
GitHub Check: Test Packages / Test ./internal/keys
GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (3)

apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.test.tsx (3)

2-2: Implementation of QueryTimeProvider mock looks good.

The addition of the QueryTimeProvider import and mock implementation correctly supports the centralized query time management that replaced local timestamp calculations. This aligns with the PR's objective to ensure consistent time intervals for data fetching across charts and logs.

Also applies to: 5-6, 13-19

161-163: Good use of test wrapper for QueryTimeProvider.

Adding the QueryTimeProvider wrapper to the renderHook call ensures that the test properly simulates the real-world environment where the hook operates within this context. This is essential for accurate testing of time-based query behavior.

90-90:
Details

❓ Verification inconclusive

Verify the increased console error count expectation.

The number of expected console error calls has increased from 3 to 6. This might be due to additional validation triggered by the integration of useQueryTime, but it would be good to verify if this is the expected behavior.

🏁 Script executed:
#!/bin/bash
# Find the useRatelimitLogsQuery implementation to understand validation logic
echo "Searching for the implementation of useRatelimitLogsQuery..."
rg -U "export (const|function) useRatelimitLogsQuery" --type typescript -A 20
Length of output: 245

🏁 Script executed:
#!/bin/bash
set -e

echo "Searching for hook file by name patterns..."
fd use-logs-query || echo "No files matching 'use-logs-query'"
fd useLogsQuery || echo "No files matching 'useLogsQuery'"

# Pick the first non-test hook implementation file
HOOK_FILE=$(fd -t f use-logs-query -E ".test." | head -n1)
if [ -z "$HOOK_FILE" ]; then
  echo "Fallback: searching for useLogsQuery implementation..."
  HOOK_FILE=$(fd -t f useLogsQuery -E ".test." | head -n1)
fi

if [ -z "$HOOK_FILE" ]; then
  echo "ERROR: Could not locate the hook implementation file."
  exit 1
fi

echo "Found hook file: $HOOK_FILE"
echo
echo "===== Function signature and surrounding lines ====="
rg "export (const|function) useLogsQuery" -A 10 -n "$HOOK_FILE" || echo "Function signature not found via grep"
echo
echo "===== console.error calls in hook ====="
rg "console.error" -n "$HOOK_FILE" || echo "No console.error calls found"
Length of output: 1959
#!/bin/bash
set -e

# Locate the ratelimits test file
TEST_FILE=$(fd -t f use-logs-query.test.tsx | grep 'ratelimits')
echo "Test file: $TEST_FILE"

echo
echo "===== Test file header (1-200 lines) ====="
sed -n '1,200p' "$TEST_FILE"

echo
echo "===== consoleMock occurrences ====="
grep -n "consoleMock" "$TEST_FILE"

# Locate the ratelimits hook implementation (non-test)
HOOK_FILE=$(fd -t f 'use-logs-query.ts' | grep 'ratelimits' | grep -v '.test.' | head -n1)
echo
echo "Hook implementation file: $HOOK_FILE"

echo
echo "===== Hook implementation (1-200 lines) ====="
sed -n '1,200p' "$HOOK_FILE"

echo
echo "===== console.error calls in hook ====="
grep -n "console.error" "$HOOK_FILE"
#!/bin/bash
set -e

echo "Locating ratelimits hook implementation file..."
HOOK_FILE=$(fd -t f 'use-logs-query.ts' | grep 'ratelimits' | grep -v '.test' | head -n1)
echo "Hook file: $HOOK_FILE"

echo
echo "===== Hook implementation (1-200 lines) ====="
sed -n '1,200p' "$HOOK_FILE"

echo
echo "===== console.error occurrences in hook ====="
grep -n "console.error" "$HOOK_FILE" || echo "No console.error calls found in hook"

echo
echo "Locating ratelimits test file..."
TEST_FILE=$(fd -t f 'use-logs-query.test.tsx' | grep 'ratelimits')
echo "Test file: $TEST_FILE"

echo
echo "===== Test file (1-200 lines) ====="
sed -n '1,200p' "$TEST_FILE"

echo
echo "===== expect(consoleMock).toHaveBeenCalledTimes usages ====="
grep -n "toHaveBeenCalledTimes" "$TEST_FILE"
#!/bin/bash
set -e

# Locate ratelimits hook implementation and test files
HOOK_IMPL=$(fd -t f 'use-logs-query.ts' | grep 'ratelimits' | grep -v 'test')
TEST_FILE=$(fd -t f 'use-logs-query.test.tsx' | grep 'ratelimits')

echo "Hook implementation file: $HOOK_IMPL"
echo
echo "===== Hook implementation (lines 1–200) ====="
sed -n '1,200p' "$HOOK_IMPL"

echo
echo "===== console.error calls in hook ====="
grep -n "console.error" "$HOOK_IMPL" || echo "No console.error calls found in hook"

echo
echo "Test file: $TEST_FILE"
echo
echo "===== Test file (lines 1–200) ====="
sed -n '1,200p' "$TEST_FILE"

echo
echo "===== consoleMock occurrences and expectations ====="
grep -n "consoleMock" "$TEST_FILE" || echo "No consoleMock references found in test"
grep -n "toHaveBeenCalledTimes" "$TEST_FILE" || echo "No toHaveBeenCalledTimes expectations found"
Verify increased console error count in ratelimits logs query test

The test at
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.test.tsx:90
now expects
expect(consoleMock).toHaveBeenCalledTimes(6);
instead of the previous 3 calls. Please confirm that the additional console errors are an intentional side-effect of the new validation logic (e.g. integration of useQueryTime) and not a regression. If they’re expected, add a brief comment explaining the extra validations; otherwise, adjust the test or suppress the unintended errors.

File: …/ratelimits/[namespaceId]/logs/components/table/hooks/use-logs-query.test.tsx

Line: 90 – expect(consoleMock).toHaveBeenCalledTimes(6);

✨ Finishing Touches

📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

github-actions · 2025-04-24T10:31:17Z

Thank you for following the naming conventions for pull request titles! 🙏

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

apps/dashboard/app/(app)/apis/[apiId]/_overview/components/charts/line-chart/hooks/use-fetch-timeseries.ts (1)

2-2: Validate and centralize the window constant source.

You’ve swapped in HISTORICAL_DATA_WINDOW from @/components/logs/constants, which couples this hook to the logs module. Since this same constant is now used across multiple chart hooks (APIs, logs, ratelimits), consider moving it to a shared dashboard constants file (e.g. @/components/dashboard/constants.ts) to improve cohesion and reduce cross-folder dependencies.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c7a3f70 and efc832a.

📒 Files selected for processing (5)

apps/dashboard/app/(app)/apis/[apiId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts (2 hunks)
apps/dashboard/app/(app)/apis/[apiId]/_overview/components/charts/line-chart/hooks/use-fetch-timeseries.ts (2 hunks)
apps/dashboard/app/(app)/logs/components/charts/hooks/use-fetch-timeseries.ts (2 hunks)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts (2 hunks)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/charts/hooks/use-fetch-timeseries.ts (2 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (5)

apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts (1)

apps/dashboard/components/logs/constants.ts (1)

HISTORICAL_DATA_WINDOW (2-2)

apps/dashboard/app/(app)/logs/components/charts/hooks/use-fetch-timeseries.ts (1)

apps/dashboard/components/logs/constants.ts (1)

HISTORICAL_DATA_WINDOW (2-2)

apps/dashboard/app/(app)/apis/[apiId]/_overview/components/charts/line-chart/hooks/use-fetch-timeseries.ts (1)

apps/dashboard/components/logs/constants.ts (1)

HISTORICAL_DATA_WINDOW (2-2)

apps/dashboard/app/(app)/apis/[apiId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts (1)

apps/dashboard/components/logs/constants.ts (1)

HISTORICAL_DATA_WINDOW (2-2)

apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/charts/hooks/use-fetch-timeseries.ts (1)

apps/dashboard/components/logs/constants.ts (1)

HISTORICAL_DATA_WINDOW (2-2)

⏰ Context from checks skipped due to timeout of 90000ms (8)

GitHub Check: Test Packages / Test ./packages/cache
GitHub Check: Test Packages / Test ./packages/hono
GitHub Check: Test Packages / Test ./packages/api
GitHub Check: Test Packages / Test ./internal/clickhouse
GitHub Check: Test Packages / Test ./packages/rbac
GitHub Check: Test Packages / Test ./internal/encryption
GitHub Check: autofix
GitHub Check: Analyze (javascript-typescript)

🔇 Additional comments (4)

apps/dashboard/app/(app)/apis/[apiId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts (1)

2-2: Appropriate change to standardize data window constants

The switch from TIMESERIES_DATA_WINDOW to HISTORICAL_DATA_WINDOW aligns the time window used for chart data with the logs data window, which will fix the problem where charts and logs showed different data counts. This consistency ensures that users see matching counts across the dashboard.

Also applies to: 16-16

apps/dashboard/app/(app)/ratelimits/[namespaceId]/_overview/components/charts/bar-chart/hooks/use-fetch-timeseries.ts (1)

2-2: Consistent update of time window constant

Changing to HISTORICAL_DATA_WINDOW (which equals 12 hours in milliseconds) standardizes the time window used for fetching data across all dashboard components, resolving the data count mismatch issue described in the PR.

Also applies to: 16-16

apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/components/charts/hooks/use-fetch-timeseries.ts (1)

2-2: Good synchronization with other components

This change properly aligns the rate limits log charts with the same historical window used across the dashboard, ensuring consistent data presentation between charts and logs.

Also applies to: 16-16

apps/dashboard/app/(app)/logs/components/charts/hooks/use-fetch-timeseries.ts (1)

2-2: Completes the standardization across components

This update to the logs component completes the consistent application of the HISTORICAL_DATA_WINDOW constant across all dashboard views. The change ensures all components use the same 12-hour window, fixing the discrepancy between displayed chart and log counts.

Also applies to: 15-15

.../app/(app)/apis/[apiId]/_overview/components/charts/line-chart/hooks/use-fetch-timeseries.ts

chronark

oh lol, that was surprisingly simple

ogzhanolguncu · 2025-04-24T12:39:26Z

:ez_pepe:

ogzhanolguncu · 2025-04-24T12:39:51Z

that was the reason for seeing different reason on ratelimit chart and logs

ogzhanolguncu · 2025-04-24T12:40:12Z

I thought it would be better to fetch longer intervals for charts so they look full. never again 😄

fix: charts and logs data count

efc832a

This will prevent charts and logs showing the different data. Previously, logs and charts were using different intervals for the default fetch. Reason was to show more data on the chart, but that gives the wrong impression to users.

ogzhanolguncu requested review from chronark, mcstepp and perkinsjr as code owners April 24, 2025 10:31

coderabbitai bot reviewed Apr 24, 2025

View reviewed changes

.../app/(app)/apis/[apiId]/_overview/components/charts/line-chart/hooks/use-fetch-timeseries.ts Show resolved Hide resolved

fix: get rid of all TIMESERIES_DATA_WINDOW

ef6fe2b

vercel bot deployed to Preview – dashboard April 24, 2025 10:39 View deployment

tests: fix broken case

5bc1ab3

vercel bot deployed to Preview – dashboard April 24, 2025 10:54 View deployment

ogzhanolguncu mentioned this pull request Apr 24, 2025

fix: tooltip showing wrong time interval #3189

Merged

18 tasks

chronark approved these changes Apr 24, 2025

View reviewed changes

chronark added this pull request to the merge queue Apr 24, 2025

Merged via the queue into main with commit 5b1ee90 Apr 24, 2025
30 of 31 checks passed

chronark deleted the fix-chart-and-logs-data-count branch April 24, 2025 12:43

coderabbitai bot mentioned this pull request Apr 24, 2025

refactor: prevent revalidation #3191

Merged

18 tasks

Conversation

ogzhanolguncu commented Apr 24, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Type of change

How should this be tested?

Checklist

Required

Appreciated

BEFORE

AFTER

Summary by CodeRabbit

Uh oh!

changeset-bot bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

vercel bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Suggested reviewers

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

github-actions bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chronark left a comment

Choose a reason for hiding this comment

Uh oh!

ogzhanolguncu commented Apr 24, 2025

Uh oh!

ogzhanolguncu commented Apr 24, 2025

Uh oh!

ogzhanolguncu commented Apr 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ogzhanolguncu commented Apr 24, 2025 •

edited by coderabbitai bot

Loading

changeset-bot bot commented Apr 24, 2025 •

edited

Loading

vercel bot commented Apr 24, 2025 •

edited

Loading

coderabbitai bot commented Apr 24, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

github-actions bot commented Apr 24, 2025 •

edited

Loading