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

(DAL) Bulk insert user stats #2145

Merged
merged 4 commits into from
Aug 21, 2024
Merged

Conversation

nick-bisonai
Copy link
Collaborator

@nick-bisonai nick-bisonai commented Aug 19, 2024

Description

User statistics for rest calls weren't being collected after migration of framework from fiber to go/http

This adds functionality to collect user rest call stats alongside with bulk insertion every certain interval (previously inserted each entries)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • 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

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.

Deployment

  • Should publish npm package
  • Should publish Docker image

@nick-bisonai nick-bisonai self-assigned this Aug 19, 2024
Copy link
Contributor

coderabbitai bot commented Aug 19, 2024

Walkthrough

Walkthrough

The recent changes enhance the functionality of the server by integrating a statistics application. This includes updates to request logging, allowing for better monitoring and debugging of API calls. Key modifications involve alterations to constructors and handlers, enabling smoother interaction with the new logging middleware. Additionally, tests are updated to support the new functionality, ensuring robust performance tracking.

Changes

Files Summary of Changes
node/pkg/dal/apiv2/... Updated Start and NewServer to include a new statsApp parameter for logging middleware.
node/pkg/dal/apiv2/types.go Added StatsApp field to ServerV2Config and introduced WithStatsApp option. Modified handler type.
node/pkg/dal/app.go Integrated stats package; modified Run function to start statistics tracking and update API call.
node/pkg/dal/tests/main_test.go Enhanced TestItems and setup function to incorporate StatsApp for performance monitoring.
node/pkg/dal/utils/stats/stats.go Improved stats package with new configuration and logging functions; added RestEntry type.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant StatsApp
    participant Server
    participant Database

    Client->>Server: Send Request
    Server->>StatsApp: Log Request
    StatsApp-->>Server: Log Entry
    Server->>Database: Process Request
    Database-->>Server: Response
    Server->>Client: Send Response
Loading

🐇 "In the meadow, changes bloom,
With stats to track, we clear the gloom.
A hop, a skip, our server's grand,
Logging requests, a helping hand!
StatsApp shines, a guiding light,
In code we trust, all feels just right!" 🌼


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
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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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.

@nick-bisonai nick-bisonai linked an issue Aug 19, 2024 that may be closed by this pull request
@nick-bisonai nick-bisonai force-pushed the feat/bulk-insert-user-stats branch from ebbd073 to 5629c05 Compare August 20, 2024 04:01
@nick-bisonai nick-bisonai marked this pull request as ready for review August 20, 2024 07:50
@nick-bisonai nick-bisonai requested a review from a team as a code owner August 20, 2024 07:50
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 987e728 and 336fe59.

Files selected for processing (5)
  • node/pkg/dal/apiv2/controller.go (3 hunks)
  • node/pkg/dal/apiv2/types.go (3 hunks)
  • node/pkg/dal/app.go (3 hunks)
  • node/pkg/dal/tests/main_test.go (4 hunks)
  • node/pkg/dal/utils/stats/stats.go (4 hunks)
Additional comments not posted (32)
node/pkg/dal/apiv2/types.go (4)

9-9: Dependency on stats package added.

The addition of the stats package seems appropriate given the new functionality. Ensure that this package is correctly implemented and utilized throughout the codebase.


25-25: Refactor: Use http.Handler instead of *http.ServeMux.

Replacing serveMux with a more generic http.Handler increases flexibility in handling HTTP requests. This change aligns with best practices for middleware and request handling.


33-33: Addition of StatsApp to ServerV2Config.

Adding StatsApp enhances the server's configurability, allowing integration with a statistics application. Ensure that this field is correctly initialized and used in the application.


56-60: New function WithStatsApp added.

This functional option allows users to set the StatsApp field in ServerV2Config, enhancing configurability. Ensure that this option is used where necessary to configure StatsApp.

node/pkg/dal/app.go (3)

14-14: Dependency on stats package added.

The addition of the stats package is necessary for the new functionality. Ensure that the package is correctly implemented and utilized throughout the codebase.


25-26: Initialize and manage statsApp lifecycle.

The statsApp is correctly initialized and stopped using defer, ensuring proper resource management. This approach is effective for managing the lifecycle of the statistics application.


52-52: Integration of WithStatsApp in apiv2.Start.

Passing WithStatsApp(statsApp) to apiv2.Start integrates the statistics application into the server. This change ensures that statistical data is available for operational metrics and performance monitoring.

node/pkg/dal/tests/main_test.go (5)

20-20: Dependency on stats package added.

The addition of the stats package is necessary for the new functionality in tests. Ensure that the package is correctly implemented and utilized throughout the test suite.


36-36: Addition of StatsApp to TestItems.

Including StatsApp in TestItems allows for performance monitoring during tests. This change improves the ability to diagnose issues or understand system behavior under load.


118-120: Initialize and run StatsApp in setup.

The StatsApp is correctly initialized and run in a separate goroutine, allowing it to operate concurrently with other components. Ensure that the goroutine is managed properly to avoid resource leaks.


130-130: Assign StatsApp to TestItems.

Assigning StatsApp to TestItems ensures that it is accessible for test operations and cleanup. This change is necessary for proper resource management in tests.


138-138: Stop StatsApp in cleanup.

Stopping StatsApp during cleanup ensures that all components are gracefully terminated, preventing resource leaks. This approach is effective for managing resources in tests.

node/pkg/dal/utils/stats/stats.go (17)

4-9: Imports for new functionality.

The added imports for bufio, bytes, errors, net, and net/http are necessary for the new logging and middleware functionality. Ensure these are used efficiently throughout the implementation.


34-37: Default configuration constants added.

The default values for BulkLogsCopyInterval and BufferSize are sensible and provide a good starting point for configuration. Ensure these defaults are suitable for typical use cases.


39-42: Definition of StatsAppConfig.

The StatsAppConfig struct allows for flexible configuration of the statistics application. This design is effective for managing application settings.


46-56: Functional options for StatsAppConfig.

The use of functional options (WithBulkLogsCopyInterval and WithBufferSize) provides a flexible way to configure StatsAppConfig. This pattern is well-suited for managing optional parameters.


58-62: Definition of StatsApp.

The StatsApp struct is well-designed to manage the lifecycle and operations of the statistics application. Ensure that the RestEntryBuffer is appropriately sized and managed.


68-73: Definition of RestEntry.

The RestEntry struct encapsulates details of API calls, which is crucial for logging. Ensure that all necessary fields are captured for effective monitoring.


75-92: Implementation of NewStatsApp.

The NewStatsApp function initializes a StatsApp with default or customized settings. This approach is effective for creating configurable instances of the application.


94-98: Implementation of Start.

The Start function initializes and runs a StatsApp, ensuring it operates concurrently. This function effectively encapsulates the startup process.


100-102: Implementation of Stop.

The Stop method correctly cancels the context, ensuring that the application can be gracefully stopped. This is crucial for resource management.


104-132: Implementation of Run.

The Run method efficiently manages the periodic copying of log entries to the database. Ensure that the ticker interval and buffer handling meet performance requirements.


134-162: Implementation of RequestLoggerMiddleware.

The middleware captures and logs API request details effectively. Ensure that the logging does not introduce significant overhead or latency.


200-204: Definition of StatsLogger.

The StatsLogger type is well-designed for capturing response details. Ensure that it integrates seamlessly with existing logging mechanisms.


206-214: Implementation of NewStatsLogger.

The NewStatsLogger function initializes a StatsLogger, setting up necessary buffers and status codes. This function is crucial for logging response details.


216-219: Implementation of Write method.

The Write method logs response bodies while maintaining functionality. Ensure that this does not interfere with normal response handling.


221-223: Implementation of Header method.

The Header method correctly returns the response headers. This method is essential for middleware compatibility.


226-229: Implementation of WriteHeader method.

The WriteHeader method logs status codes and writes headers. Ensure that status codes are accurately captured.


231-236: Implementation of Hijack method.

The Hijack method allows for advanced connection handling. Ensure that this functionality is necessary and correctly implemented.

node/pkg/dal/apiv2/controller.go (3)

52-52: Integration of statsApp in Start function is appropriate.

The addition of the statsApp parameter to the NewServer call enhances the server's logging capabilities and aligns with the PR's objective to improve monitoring and debugging.


105-105: Middleware integration in ServeHTTP is correctly implemented.

The change to use s.handler instead of s.serveMux ensures all requests are processed through the logging middleware, enhancing request monitoring.


68-88: Use of RequestLoggerMiddleware enhances request logging.

The integration of the RequestLoggerMiddleware to the serveMux is a beneficial addition for monitoring HTTP requests. The refactoring to use serveMux improves code clarity.

Ensure all routes are correctly logged by the middleware.

Verification successful

All routes are correctly logged by the middleware.

The RequestLoggerMiddleware is applied to the serveMux, ensuring that all defined routes are logged. This confirms that the integration is functioning as intended.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that all routes are correctly logged by the middleware.

# Test: Search for the `RequestLoggerMiddleware` usage. Expect: All routes are wrapped by the middleware.
rg --type go $'serveMux.HandleFunc' -A 3

Length of output: 1009

Copy link
Contributor

@Intizar-T Intizar-T left a comment

Choose a reason for hiding this comment

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

lgtm!

@nick-bisonai nick-bisonai merged commit 2c35ed0 into master Aug 21, 2024
1 check passed
@nick-bisonai nick-bisonai deleted the feat/bulk-insert-user-stats branch August 21, 2024 03:11
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.

(DAL) Bulk Insert stats
2 participants