ar-io / ar-io-node

A scalable and modular gateway built for the permaweb atop the Arweave permanent data storage network.
https://ar.io
GNU Affero General Public License v3.0
97 stars 65 forks source link

feat(data-sources): add getData metrics #148

Closed karlprieb closed 3 months ago

karlprieb commented 3 months ago

Added getDataErrorsTotal, getDataStreamErrorsTotal and getDataStreamSuccesssTotal metrics for ContiguousDataSources.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 90.90909% with 29 lines in your changes missing coverage. Please review.

Project coverage is 66.24%. Comparing base (8f512d0) to head (34613a7).

Files Patch % Lines
src/data/read-through-data-cache.ts 78.50% 23 Missing :warning:
src/data/tx-chunks-data-source.ts 91.78% 6 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #148 +/- ## =========================================== + Coverage 65.12% 66.24% +1.11% =========================================== Files 27 28 +1 Lines 6578 6733 +155 Branches 305 325 +20 =========================================== + Hits 4284 4460 +176 + Misses 2293 2272 -21 Partials 1 1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

coderabbitai[bot] commented 3 months ago
Walkthrough ## Walkthrough The recent changes introduce metric tracking for data retrieval, stream success, and error events across various data source modules. Additionally, logging has been updated to use dynamic class names for more accurate logging. New utilities for mocking stream data for tests are also included, improving the reliability and clarity of test cases. ## Changes | Files Changed | Summary of Changes | |------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------| | `src/data/ar-io-data-source.test.ts` | Added imports for `axios`, `metrics`, and other utilities; updated test logic to include metric tracking and improved data handling. | | `src/data/ar-io-data-source.ts` | Imported `metrics`, updated logging, and added metric tracking for data retrieval and stream events. | | `src/data/gateway-data-source.test.ts` | Included metrics tracking and assertions; updated mock data and enhanced test cases for error handling. | | `src/data/gateway-data-source.ts` | Added `metrics` import, refactored logging, and integrated metric tracking for stream events. | | `src/data/read-through-data-cache.ts` | Imported `metrics`, improved logging, and implemented metrics for data stream events during caching operations. | | `src/data/s3-data-source.ts` | Similar changes as in other data source modules: `metrics` import, logging adjustments, and metric tracking for data stream events. | | `src/data/test-utils.ts` | Introduced `TestDestroyedReadable` class for simulating stream reading and destruction, along with mocked stream data utility. | | `src/data/tx-chunks-data-source.test.ts` | Added `metrics` and `beforeEach` imports; updated test cases with async/await syntax and new setup/teardown logic for metric assertions. | | `src/metrics.ts` | Added new metric counters for data errors and stream events to improve monitoring capabilities. | Note: `/.../` indicates truncated file paths for brevity. ## Sequence Diagram(s) ```mermaid sequenceDiagram participant Client participant DataSource participant Metrics participant Logger Client->>DataSource: Request data stream activate DataSource DataSource->>Metrics: Inc getDataStreamTotal DataSource->>Logger: Log "Data stream started" DataSource->>Client: Send data chunks Client->>DataSource: Process data chunks alt Data retrieval success DataSource->>Metrics: Inc getDataStreamSuccessesTotal DataSource->>Logger: Log "Data stream successful" else Data retrieval error DataSource->>Metrics: Inc getDataStreamErrorsTotal DataSource->>Logger: Log "Data stream error" end deactivate DataSource ```

Recent review details **Configuration used: CodeRabbit UI** **Review profile: CHILL**
Commits Files that changed from the base of the PR and between b5485ff22a5a28fc8ba93ad2dde02f8c11d7f1ba and 34613a777018986887a16e028997239582eb7b82.
Files selected for processing (12) * src/arweave/composite-client.ts (2 hunks) * src/data/ar-io-data-source.test.ts (8 hunks) * src/data/ar-io-data-source.ts (5 hunks) * src/data/gateway-data-source.test.ts (9 hunks) * src/data/gateway-data-source.ts (3 hunks) * src/data/read-through-data-cache.test.ts (4 hunks) * src/data/read-through-data-cache.ts (3 hunks) * src/data/s3-data-source.ts (3 hunks) * src/data/sequential-data-source.ts (1 hunks) * src/data/tx-chunks-data-source.test.ts (6 hunks) * src/data/tx-chunks-data-source.ts (3 hunks) * src/metrics.ts (1 hunks)
Files not summarized due to errors (2) * src/metrics.ts: Error: Server error. Please try again later. * src/data/ar-io-data-source.ts: Error: Server error. Please try again later.
Additional context used
GitHub Check: codecov/patch
src/data/tx-chunks-data-source.ts
[warning] 102-106: src/data/tx-chunks-data-source.ts#L102-L106 Added lines #L102 - L106 were not covered by tests
src/data/read-through-data-cache.ts
[warning] 204-209: src/data/read-through-data-cache.ts#L204-L209 Added lines #L204 - L209 were not covered by tests --- [warning] 214-219: src/data/read-through-data-cache.ts#L214-L219 Added lines #L214 - L219 were not covered by tests --- [warning] 244-249: src/data/read-through-data-cache.ts#L244-L249 Added lines #L244 - L249 were not covered by tests
Biome
src/data/read-through-data-cache.test.ts
[error] 112-112: Unexpected empty object pattern. (lint/correctness/noEmptyPattern) --- [error] 167-184: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead. --- [error] 186-222: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead. --- [error] 226-285: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead. --- [error] 287-327: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead. --- [error] 329-397: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead. --- [error] 399-445: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead. --- [error] 225-446: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead. --- [error] 34-447: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead.
src/arweave/composite-client.ts
[error] 272-272: Avoid the delete operator which can impact performance. (lint/performance/noDelete) Unsafe fix: Use an undefined assignment instead. --- [error] 278-278: Avoid the delete operator which can impact performance. (lint/performance/noDelete) Unsafe fix: Use an undefined assignment instead. --- [error] 435-435: Avoid the delete operator which can impact performance. (lint/performance/noDelete) Unsafe fix: Use an undefined assignment instead.
Additional comments not posted (34)
src/data/sequential-data-source.ts (1)
`38-38`: Dynamic class name in logs enhances maintainability.
src/data/gateway-data-source.ts (3)
`30-30`: Dynamic class name in logs and metrics integration are good practices for scalability and maintainability. Also applies to: 47-47 --- `95-105`: Properly handle potential undefined values in `requestAttributesHeaders` to prevent runtime errors. ```diff - 'ar-io-hops': requestAttributesHeaders?.attributes.hops, + 'ar-io-hops': requestAttributesHeaders?.attributes?.hops || 'default_value', ``` > Likely invalid or redundant comment. --- `95-105`: Stream event handling logic looks robust. Consider adding a test to cover these scenarios.
Verification successful --- The search results indicate that there are test cases covering the `metrics.getDataStreamErrorsTotal.inc` method, which suggests that the stream handling logic is indeed being tested. - `src/data/gateway-data-source.test.ts`: Found references to `metrics.getDataStreamErrorsTotal.inc`.
Scripts executed The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Description: Verify that stream handling logic is covered by tests. # Test: Search for the test coverage report. Expect: Coverage for lines 95-105. rg --type python $'metrics.getDataStreamErrorsTotal.inc' 'src/data/gateway-data-source.test.ts' ``` Length of output: 123 --- --- Script: ```shell #!/bin/bash # Description: Verify that stream handling logic is covered by tests. # Test: Search for the relevant test cases. Expect: Coverage for the stream handling logic. rg 'metrics.getDataStreamErrorsTotal.inc' 'src/data/gateway-data-source.test.ts' ``` Length of output: 222
src/data/tx-chunks-data-source.ts (2)
`30-30`: Dynamic class name in logs is a good practice. Ensure consistency across the application. Also applies to: 46-46 --- `60-137`: Complex stream handling logic should be covered by unit tests to ensure reliability.
Verification successful --- The search results indicate that there are indeed references to `metrics.getDataStreamErrorsTotal.inc` and `metrics.getDataStreamSuccessesTotal.inc` in the test file `src/data/tx-chunks-data-source.test.ts`. This suggests that the stream handling logic is being tested. - : The stream handling logic in `src/data/tx-chunks-data-source.ts` is covered by unit tests in `src/data/tx-chunks-data-source.test.ts`.
Scripts executed The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Description: Verify that stream handling logic is covered by tests. # Test: Search for the test coverage report. Expect: Coverage for lines 88-124. rg --type python $'metrics.getDataStreamErrorsTotal.inc' 'src/data/tx-chunks-data-source.test.ts' ``` Length of output: 125 --- --- Script: ```shell #!/bin/bash # Description: Verify that stream handling logic is covered by tests. # Test: Search for test cases related to stream handling logic. rg 'metrics.getDataStreamErrorsTotal.inc' 'src/data/tx-chunks-data-source.test.ts' rg 'metrics.getDataStreamSuccessesTotal.inc' 'src/data/tx-chunks-data-source.test.ts' ``` Length of output: 614
Tools
GitHub Check: codecov/patch
[warning] 102-106: src/data/tx-chunks-data-source.ts#L102-L106 Added lines #L102 - L106 were not covered by tests
src/data/s3-data-source.ts (2)
`29-29`: Integration of metrics and dynamic logging enhances the robustness of the S3 data source. Also applies to: 53-53 --- `136-149`: Stream handling with metrics tracking is implemented correctly. Ensure this is covered by unit tests.
Verification successful --- The search results indicate that the metrics tracking for `getDataStreamErrorsTotal.inc` and `getDataStreamSuccessesTotal.inc` is indeed covered by unit tests, as evidenced by multiple mock calls in the test files. - **Files with relevant test cases**: - `src/data/ar-io-data-source.test.ts` - `src/data/gateway-data-source.test.ts` - `src/data/read-through-chunk-data-cache.test.ts` - `src/data/read-through-data-cache.test.ts` - `src/data/tx-chunks-data-source.test.ts` No further action is required.
Scripts executed The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Description: Verify that stream handling logic is covered by tests. # Test: Search for the test coverage report. Expect: Coverage for lines 136-149. rg --type python $'metrics.getDataStreamErrorsTotal.inc' 'src/data/s3-data-source.test.ts' ``` Length of output: 118 --- --- Script: ```shell #!/bin/bash # Description: Verify that stream handling logic is covered by tests. # Test: Search for the test coverage in the TypeScript test file for metrics tracking. rg 'metrics.getDataStreamErrorsTotal.inc' 'src/data/s3-data-source.test.ts' rg 'metrics.getDataStreamSuccessesTotal.inc' 'src/data/s3-data-source.test.ts' ``` Length of output: 294 --- --- Script: ```shell #!/bin/bash # Description: Locate the test file and verify that stream handling logic is covered by tests. # Step 1: Find the TypeScript test files. fd --extension ts --glob '*test.ts' # Step 2: Search for the test coverage in the identified TypeScript test file for metrics tracking. fd --extension ts --glob '*test.ts' --exec rg 'metrics.getDataStreamErrorsTotal.inc' {} fd --extension ts --glob '*test.ts' --exec rg 'metrics.getDataStreamSuccessesTotal.inc' {} ``` Length of output: 2736
src/metrics.ts (1)
`178-194`: LGTM! The new metrics for data errors and data stream events are correctly defined.
src/data/tx-chunks-data-source.test.ts (2)
`54-57`: The mocking of metrics methods is correctly implemented in the `beforeEach` function. --- `79-87`: Ensure that the metrics are incremented correctly in the test cases for both error and success scenarios. Also applies to: 105-112, 144-151, 178-181
src/data/ar-io-data-source.ts (2)
`64-64`: The dynamic class name in logging is correctly implemented, enhancing the traceability of logs. --- `193-195`: The metrics for data errors and stream events are correctly incremented upon failures and successes. Also applies to: 216-218, 238-240, 244-246
src/data/read-through-data-cache.ts (1)
`151-160`: The metrics for data stream errors and successes are correctly incremented in the stream event handlers. Also applies to: 229-237
src/data/ar-io-data-source.test.ts (6)
`21-27`: Imports for axios, ArIO SDK, and the new test utilities are correctly added to support the new metrics and mocking functionalities. --- `26-27`: The addition of `metrics` and test utilities (`TestDestroyedReadable`, `axiosStreamData`) is appropriate for the new metrics tracking and stream data mocking. Ensure that these utilities are adequately covered in the test cases. --- `61-63`: The mocking of metric increment methods is correctly implemented. This setup is essential for verifying that metrics are accurately incremented during tests. --- Line range hint `106-133`: Proper handling of streaming data and metrics verification in the `getData` method. It's good to see the metrics being verified alongside the data integrity checks. --- Line range hint `138-192`: The retry logic with a different peer if the first one fails is well-implemented. The metrics for errors and successes are correctly updated based on the outcomes, which is crucial for accurate monitoring. --- `194-222`: The test case for incrementing `getDataStreamErrorsTotal` is well-constructed. Catching the stream destruction and verifying the metric increment is a vital part of ensuring the robustness of error handling.
src/data/gateway-data-source.test.ts (6)
`24-25`: The imports for `metrics` and the test utilities are correctly added, which are essential for the new functionality in testing data fetching from the gateway. --- `54-56`: Mocking of the metric methods is set up correctly. This preparation is crucial for ensuring that the metrics are being correctly incremented during the test scenarios. --- Line range hint `79-106`: The test for fetching data successfully from the gateway and verifying the correct increment of success metrics is well-implemented. It ensures that both data integrity and metric tracking are being handled correctly. --- `116-124`: Handling of unexpected status codes with appropriate metric increments for errors is a good practice. This ensures that all error states are captured and logged correctly. --- `138-146`: The test for handling Axios errors is well done. It correctly increments the error metrics and ensures that network errors are gracefully handled. --- `151-179`: The increment of `getDataStreamErrorsTotal` in the scenario of a destroyed stream is correctly tested. This is crucial for monitoring the robustness of the stream handling in the application.
src/data/read-through-data-cache.test.ts (6)
`31-32`: Correct addition of imports for `metrics` and `TestDestroyedReadable`. These are crucial for the new functionalities related to metrics tracking and handling of test streams. --- `124-126`: Proper setup for mocking metric methods. This setup is essential for ensuring the metrics are accurately incremented during various cache operations in the tests. --- `267-284`: The test for successfully fetching cached data and verifying the increments of success metrics is well-implemented. It ensures that the cache functionality is robust and that metrics tracking is accurate. --- `287-326`: Testing for incrementing `getDataStreamErrorsTotal` when dealing with broken cached data streams is crucial. This ensures that error handling mechanisms are robust and that all error states are correctly monitored.
Tools
Biome
[error] 287-327: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead.
--- `375-390`: The test for fetching data from the source when it's not available in the cache and ensuring that the success metrics are correctly incremented is well-done. It verifies the functionality of data retrieval and caching under expected conditions. --- `399-444`: Correct testing of error handling and metric incrementation for broken non-cached data streams. This is essential for ensuring that the system's resilience and error tracking capabilities are up to standard.
Tools
Biome
[error] 399-445: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction) Function expressions that don't use this can be turned into arrow functions. Safe fix: Use an arrow function instead.
src/arweave/composite-client.ts (2)
`155-155`: The use of `this.constructor.name` for dynamic class identification in logging is a good practice. It ensures that logs are accurately tagged with the class name, which can be helpful for debugging and monitoring. [APROVED] --- `638-682`: The method for fetching transaction data and handling potential errors with appropriate metric increments is well-implemented. It ensures that data retrieval is robust and that all operational states are monitored.
--- 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](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)
Tips ### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `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 Configration 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](https://docs.coderabbit.ai/guides/configure-coderabbit) 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](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.