refactor: query audit logs from planetscale

[chronark]

Step 2 / 3: Reading audit logs from planetscale, instead of tinybird

Next up: removing writes to tinybird

Summary by CodeRabbit

• New Features

  • Enhanced audit log retrieval process with improved filtering options based on event types and timestamps.
  • Updated structure of audit logs for better clarity and efficiency.

• Bug Fixes

  • Streamlined logic for loading and checking the existence of logs.

• Documentation

  • Updated type definitions and relationships for audit logs to reflect new structures.

[vercel]

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	Oct 4, 2024 5:15pm
play	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 4, 2024 5:15pm
unkey-engineering	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 4, 2024 5:15pm
www	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 4, 2024 5:15pm

2 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
planetfall	⬜️ Ignored (Inspect)	Visit Preview		Oct 4, 2024 5:15pm
workflows	⬜️ Ignored (Inspect)	Visit Preview		Oct 4, 2024 5:15pm

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9a4e209

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

[coderabbitai]

Warning

Rate limit exceeded

@chronark has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 18 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Files that changed from the base of the PR and between 09508b7 and 9a4e209.

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

📝 Walkthrough

Walkthrough

The changes involve significant modifications to the AuditPage and Row components, as well as the introduction of a new export in the database schema. The AuditPage now retrieves audit logs through a new database query, incorporating a revised data structure and filtering mechanism. The Row component reflects these changes by updating the auditLog type definition. Additionally, a new export defines relationships for the auditLogBucket table, establishing connections with the auditLog and workspaces tables.

Changes

File Path	Change Summary
apps/dashboard/app/(app)/audit/[bucket]/page.tsx	Updated AuditPage to retrieve logs via a new database query; modified log structure and filtering; updated AuditLogTable component.
apps/dashboard/app/(app)/audit/[bucket]/row.tsx	Restructured auditLog type definition; renamed resources to targets; changed actor.type to a generic string.
internal/db/src/schema/audit_logs.ts	Added new export auditLogBucketRelations to define relationships between auditLogBucket, auditLog, and workspaces.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant AuditPage
    participant Database
    participant AuditLogTable

    User->>AuditPage: Request audit logs
    AuditPage->>Database: Fetch audit logs by workspace ID and bucket
    Database-->>AuditPage: Return filtered audit logs
    AuditPage->>AuditLogTable: Pass logs to render
    AuditLogTable-->>User: Display logs

Loading

Possibly related PRs

• feat: mostly just duplicate audit logs and other clickhouse stuff #2134: The changes in this PR involve modifications to the logging functions related to audit logs, which may connect to the new structure and retrieval methods for audit logs introduced in the main PR.

Suggested reviewers

• mcstepp
• perkinsjr

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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 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]

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

[coderabbitai]

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (5)

apps/dashboard/app/(app)/audit/[bucket]/row.tsx (2)

22-25: LGTM! Consider adding a comment for clarity.

The renaming of resources to targets is consistent and aligns with the PR objectives. The functionality remains unchanged.

Consider adding a brief comment explaining the reason for this change, e.g.:

// 'targets' represents the affected entities in PlanetScale audit logs
targets: {
  type: string;
  id: string;
}[];

This would help future developers understand the context of this property.

Also applies to: 102-103

---

Line range hint 1-114: Overall, the changes align with the PR objectives, but there are a few points to address.

The modifications to this file are part of the transition from Tinybird to PlanetScale for audit logs. The main changes involve renaming resources to targets and broadening the type of actor.type. While these changes are consistent within the file, there are a couple of points to consider:

1. The broadening of actor.type to a generic string might reduce type safety.
2. Adding a comment to explain the targets property would improve code clarity.

These minor adjustments would enhance the overall quality and maintainability of the code while keeping it aligned with the PR objectives.

internal/db/src/schema/audit_logs.ts (2)

43-49: LGTM! Consider a minor improvement for consistency.

The new auditLogBucketRelations export correctly defines the relationships for the auditLogBucket table, aligning with the existing schema structure and supporting the transition to PlanetScale for reading audit logs. The implementation follows Drizzle ORM conventions and best practices.

For consistency with other relation definitions in this file, consider adding explicit field and reference definitions for the logs relation. This would make the relationship more explicit and easier to maintain. Here's a suggested modification:

 export const auditLogBucketRelations = relations(auditLogBucket, ({ one, many }) => ({
   workspace: one(workspaces, {
     fields: [auditLogBucket.workspaceId],
     references: [workspaces.id],
   }),
-  logs: many(auditLog),
+  logs: many(auditLog, {
+    fields: [auditLogBucket.id],
+    references: [auditLog.bucketId],
+  }),
 }));

This change would make the logs relation consistent with the workspace relation and other relation definitions in the file.

---

Line range hint 1-49: Overall changes align well with PR objectives.

The addition of auditLogBucketRelations supports the transition from Tinybird to PlanetScale for reading audit logs, which is the main objective of this PR. The existing schema structure is maintained, ensuring backwards compatibility.

As this is step 2 of 3 in the refactoring process, with the next step involving eliminating write operations to Tinybird, consider the following recommendations for the next phase:

1. Ensure that any code relying on Tinybird-specific features for write operations is identified and prepared for refactoring.
2. Plan for a gradual transition of write operations, possibly implementing a dual-write strategy temporarily to ensure data consistency during the migration.
3. Update any relevant documentation or comments in the code to reflect the ongoing transition from Tinybird to PlanetScale.

These considerations will help smooth the transition in the upcoming final step of the refactoring process.

apps/dashboard/app/(app)/audit/[bucket]/page.tsx (1)

135-135: Remove or restore the commented out ExportCsv component

The ExportCsv component is currently commented out. If the export functionality is no longer needed, consider removing the commented code to keep the codebase clean. If you plan to reintroduce it later, adding a TODO comment might be helpful.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e7ac47f and e64b4f3.

📒 Files selected for processing (3)

• apps/dashboard/app/(app)/audit/[bucket]/page.tsx (9 hunks)
• apps/dashboard/app/(app)/audit/[bucket]/row.tsx (2 hunks)
• internal/db/src/schema/audit_logs.ts (1 hunks)

🔇 Additional comments (12)

apps/dashboard/app/(app)/audit/[bucket]/row.tsx (1)

17-17: Verify the intention behind changing actor.type to a generic string.

The type of auditLog.actor.type has been changed from a union of string literals ("key" | "user" | "system") to a generic string. This reduces type safety and could potentially lead to runtime errors if unexpected values are used.

Could you please clarify the reasoning behind this change? If it's intentional to accommodate a wider range of actor types, consider using a union type that includes both the original literals and a string template literal for custom types, e.g., type ActorType = "key" | "user" | "system" | custom_${string};. This would maintain some level of type safety while allowing for extensibility.

apps/dashboard/app/(app)/audit/[bucket]/page.tsx (11)

67-67: Retention cutoff timestamp calculated correctly

The calculation for retentionCutoffUnixMilli is accurate and correctly factors in the retention days to determine the cutoff time.

---

69-69: Combining actor IDs for filtering logs

The selected actor IDs are properly combined from selectedRootKeys and selectedUsers, which will be used for filtering the logs.

---

71-88: Querying audit logs with appropriate filters

The query for retrieving the audit logs is well-constructed, applying the necessary filters based on workspace ID, bucket name, selected events, retention cutoff, and selected actor IDs. Including the targets in the with clause and ordering the logs by id ensures comprehensive data retrieval.

---

118-120: Event options correctly generated for 'unkey_mutations' bucket

The event options are accurately generated from unkeyAuditLogEvents.Values, ensuring that the filter includes all relevant events for the 'unkey_mutations' bucket.

---

122-124: Event options specified for rate limit buckets

The filter options for rate limit events are correctly specified, providing users with appropriate choices for 'ratelimit.success' and 'ratelimit.denied'.

---

160-176: Transforming logs data for AuditLogTable component

The logs are correctly transformed and mapped to the structure expected by the AuditLogTable component. All necessary fields are included.

---

193-209: Defining the logs prop type accurately

The logs prop type for the AuditLogTable component is well-defined, ensuring type safety and clarity of the expected data structure.

---

214-214: Appropriate handling of empty log results

The component correctly displays an empty state with helpful messaging when no logs are available, enhancing user experience.

---

262-262: Fetching unique user IDs for actor information

The extraction of unique user IDs from the logs is correctly implemented, ensuring that user details are fetched without duplication.

---

286-306: Rendering log entries with user information

The log entries are properly rendered in the table, including user details when available. The mapping ensures that all necessary information is displayed to the user.

---

316-316: Pagination handled correctly with 'Load more' functionality

The 'Load more' button correctly uses the time of the last log entry to fetch additional logs before that timestamp. The logic for enabling or disabling the button based on hasMoreLogs is appropriate.

[coderabbitai]

⚠️ Potential issue

Add a limit to the logs query to optimize performance

Currently, hasMoreLogs is determined by checking if logs.length >= 100, implying that the logs query fetches up to 100 records. However, the query does not specify a limit, which might lead to performance issues with large datasets. Consider adding a limit to the query to ensure consistent performance and accurate pagination.

Apply this diff to add a limit to the logs query:

logs: {
  where: ...,
  with: ...,
  orderBy: ...,
+ limit: 100,
},

This change aligns the query with the pagination logic based on hasMoreLogs.

Committable suggestion was skipped due to low confidence.

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)

apps/dashboard/app/(app)/audit/[bucket]/page.tsx (4)

67-70: LGTM! Consider adding a comment for clarity.

The introduction of retentionCutoffUnixMilli and selectedActorIds improves the code structure and aligns with the workspace's audit log retention policy. These changes support the transition to querying audit logs from PlanetScale.

Consider adding a brief comment explaining the purpose of selectedActorIds:

// Combine root keys and users for efficient actor filtering in the query
const selectedActorIds = [...selectedRootKeys, ...selectedUsers];

---

71-89: LGTM! Consider adding an index for query optimization.

The direct query to the auditLogBucket table aligns with the PR objective of transitioning to PlanetScale for reading audit logs. The query structure efficiently filters logs based on events, timestamps, and actor IDs while including necessary relations.

To optimize query performance, consider adding a composite index on the auditLogBucket table for frequently used filter combinations. For example:

CREATE INDEX idx_audit_log_bucket_filters ON auditLogBucket (workspaceId, name, event, createdAt, actorId);

This index can significantly improve query performance, especially as the volume of audit logs grows.

---

160-176: LGTM! Consider using a type alias for improved maintainability.

The updated structure of the logs passed to the AuditLogTable component aligns well with the new audit log retrieval process from PlanetScale. This change ensures that all necessary information is correctly passed to the component.

To improve type safety and maintainability, consider defining a type alias for the log entry structure:

type AuditLogEntry = {
  id: string;
  event: string;
  time: number;
  actor: {
    id: string;
    name: string | null;
    type: string;
  };
  location: string | null;
  description: string;
  targets: Array<{
    id: string;
    type: string;
    name: string | null;
  }>;
};

// Then use it in the mapping:
logs={bucket.logs.map((l): AuditLogEntry => ({
  // ... (rest of the mapping remains the same)
}))}

This approach will make it easier to maintain consistency between the log structure and its usage throughout the component.

---

193-209: LGTM! Consider using the suggested type alias for consistency.

The updated type definition for the logs prop in the AuditLogTable component accurately reflects the new structure of the log entries. This change improves type safety and code readability.

For consistency with the previous suggestion, consider using the AuditLogEntry type alias here as well:

const AuditLogTable: React.FC<{
  selectedEvents: string[];
  selectedUsers: string[];
  selectedRootKeys: string[];
  before?: number;
  logs: AuditLogEntry[];
}> = async ({ selectedEvents, selectedRootKeys, selectedUsers, before, logs }) => {
  // ... (rest of the component implementation)
};

This change will maintain consistency throughout the component and make it easier to update the log entry structure in the future if needed.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between e64b4f3 and 09508b7.

📒 Files selected for processing (1)

• apps/dashboard/app/(app)/audit/[bucket]/page.tsx (9 hunks)

🔇 Additional comments (4)

apps/dashboard/app/(app)/audit/[bucket]/page.tsx (4)

90-98: Handle missing bucket with a user-friendly message

A previous review comment suggested improving this section to display a more informative message when the bucket is not found. That suggestion is still valid and applicable to the current code.

---

135-135: Clarify the status of the ExportCsv functionality

The ExportCsv component is currently commented out. Is this a temporary measure, or should this functionality be removed entirely? If it's temporary, consider adding a TODO comment explaining the reason and when it's expected to be re-enabled.

Could you provide more context on the plans for the ExportCsv functionality?

---

214-214: LGTM! Simplified empty logs check.

The updated condition for checking if there are no logs is more straightforward and aligns well with the new structure of the logs prop. This change improves code readability and maintainability.

---

Line range hint 239-306: LGTM! Consider parameterizing the page size.

The updates to the user retrieval logic and log entry rendering align well with the new structure of logs retrieved from PlanetScale. The code efficiently handles potential missing users and correctly displays the log information.

Consider parameterizing the page size used in the hasMoreLogs check:

const PAGE_SIZE = 100;
const hasMoreLogs = logs.length >= PAGE_SIZE;

This change would make it easier to adjust the page size in the future if needed, and it would make the code more self-documenting. Ensure that this page size is consistent with any limits set in the database query.

Could you confirm that the page size of 100 is intentional and matches any limits set in the database query? If not, consider adjusting it to match the actual query limit.