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

feat: Add Prometheus metrics for processing time before and after sending relay #1543

Merged
merged 12 commits into from
Jul 23, 2024

Conversation

oren-lava
Copy link
Collaborator

@oren-lava oren-lava commented Jul 7, 2024

Description

Closes: #XXXX

In this PR I added two Prometheus metrics: processing time measurement before sending a relay to the provider, and a processing time measurement after the relay returns to the consumer for the provider. The metrics goal is to benchmark the consumer's performance on the relay transmission workflow without the provider processing time.

Local tests show that the consumer's overhead is only a couple of milliseconds.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • read the contribution guide
  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the main branch
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

  • New Features

    • Introduced processing timestamp and provider processing time measurement in the analytics metrics.
    • Added methods to handle processing latency metrics before and after provider interaction.
  • Improvements

    • Enhanced relay sending process with detailed analytics for better performance tracking.
    • Improved metrics collection capabilities in gRPC and JSON-RPC services.
  • Refactor

    • Updated relay-related methods to include enhanced analytics parameters.

@oren-lava oren-lava self-assigned this Jul 7, 2024
Copy link
Contributor

coderabbitai bot commented Jul 7, 2024

Walkthrough

The update primarily restructures metrics handling within the relay processing flow, enhancing the tracking of processing latency before and after provider interactions. Key changes involve the addition of new fields and methods to the RelayMetrics struct, as well as modifications to several methods in the RPCConsumerServer struct to incorporate these new metrics functionalities, promoting better analytics capabilities overall.

Changes

File/Path Change Summary
protocol/metrics/analytics.go Expanded RelayMetrics struct with new fields for timestamps and added methods for processing control.
protocol/metrics/rpcconsumerlogs.go Introduced methods for logging processing latency before and after provider interactions.
protocol/rpcconsumer/rpcconsumer_server.go Updated methods to include analytics *metrics.RelayMetrics parameter for improved metrics tracking.
protocol/chainlib/grpc.go Enhanced Serve method to include new metrics collection calls.
protocol/chainlib/jsonRPC.go Modified Serve method to log processing time and latency effectively.
protocol/chainlib/rest.go Improved handling of processing latency metrics and error management in Serve.
protocol/chainlib/tendermintRPC.go Updated Serve method to enhance logging and metrics collection.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant RPCConsumerServer
    participant Provider
    participant Metrics

    Client->>RPCConsumerServer: SendRelay(request)
    RPCConsumerServer->>Metrics: Create RelayMetrics
    RPCConsumerServer->>RPCConsumerServer: ProcessRelaySend(request, analytics)
    RPCConsumerServer->>Metrics: AddMetricForProcessingLatencyBeforeProvider
    RPCConsumerServer->>Provider: sendRelayToProvider(request, analytics)
    Provider-->>RPCConsumerServer: response
    RPCConsumerServer->>Metrics: AddMetricForProcessingLatencyAfterProvider
    RPCConsumerServer-->>Client: SendRelay(response)
Loading

Poem

Metrics whisper in the code,
Tracking times where data flowed.
Relays now with timestamps bright,
Measure latencies, day and night.
In the logs, new methods bloom,
Bringing clarity to the room.
Rabbits dance, in fields of nodes! 🌟🐇


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.

Copy link

github-actions bot commented Jul 7, 2024

Test Results

2 016 tests  ±0   2 016 ✅ +1   32m 33s ⏱️ - 1m 14s
  142 suites ±0       0 💤 ±0 
    8 files   ±0       0 ❌  - 1 

Results for commit 065e9fb. ± Comparison against base commit 6282a32.

♻️ This comment has been updated with latest results.

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: 5

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between d6f4c5c and c896e97.

Files selected for processing (8)
  • protocol/chainlib/grpc.go (2 hunks)
  • protocol/chainlib/jsonRPC.go (2 hunks)
  • protocol/chainlib/rest.go (4 hunks)
  • protocol/chainlib/tendermintRPC.go (4 hunks)
  • protocol/metrics/analytics.go (2 hunks)
  • protocol/metrics/metrics_consumer_manager.go (7 hunks)
  • protocol/metrics/rpcconsumerlogs.go (1 hunks)
  • protocol/rpcconsumer/rpcconsumer_server.go (10 hunks)
Additional comments not posted (21)
protocol/metrics/analytics.go (1)

52-58: LGTM!

The function SetProcessingTimestamp correctly sets the ProcessingTimestamp field if the RelayMetrics instance is not nil.

protocol/metrics/rpcconsumerlogs.go (2)

157-161: LGTM!

The function AddMetricForProcessingLatencyBeforeProvider correctly checks if the analytics parameter is not nil and if the ProcessingTimestamp is valid before setting the relay processing latency before the provider.


163-167: LGTM!

The function AddMetricForProcessingLatencyAfterProvider correctly checks if the analytics parameter is not nil and if the ProcessingTimestamp is valid before setting the relay processing latency after the provider.

protocol/metrics/metrics_consumer_manager.go (5)

54-56: LGTM!

The new fields relayProcessingLatencyBeforeProvider, relayProcessingLatencyAfterProvider, and averageProcessingLatency in the ConsumerMetricsManager struct are correctly defined and initialized.

Also applies to: 213-215


267-274: LGTM!

The function SetRelayProcessingLatencyBeforeProvider correctly checks if the ConsumerMetricsManager instance is not nil, updates the relay processing latency before the provider, and sets the updated latency in the Prometheus gauge.


276-283: LGTM!

The function SetRelayProcessingLatencyAfterProvider correctly checks if the ConsumerMetricsManager instance is not nil, updates the relay processing latency after the provider, and sets the updated latency in the Prometheus gauge.


285-293: LGTM!

The function updateRelayProcessingLatency correctly updates the average processing latency for a given key and returns the updated average latency in milliseconds.


347-353: LGTM!

The function getKeyForProcessingLatency correctly generates a key for processing latency based on the chain ID, API interface, and whether the latency is before or after the provider.

protocol/chainlib/rest.go (2)

311-311: LGTM!

The addition of SetProcessingTimestamp and AddMetricForProcessingLatencyAfterProvider calls in the Serve method of RestChainListener correctly tracks processing latencies before and after sending relay.

Also applies to: 346-349


364-364: LGTM!

The addition of SetProcessingTimestamp and AddMetricForProcessingLatencyAfterProvider calls in the handlerUse method of RestChainListener correctly tracks processing latencies before and after sending relay.

Also applies to: 421-423

protocol/chainlib/grpc.go (2)

321-321: Add error handling for SetProcessingTimestamp.

Ensure that metricsData.SetProcessingTimestamp(startTime) is correctly executed without errors.

Verification successful

Add error handling for SetProcessingTimestamp.

The SetProcessingTimestamp function includes a nil check to handle potential errors. No further error handling is required.

  • protocol/metrics/analytics.go, lines 52-58
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `SetProcessingTimestamp` is correctly executed without errors.

# Test: Search for the function definition. Expect: No errors in the implementation.
ast-grep --lang go --pattern $'func (rm *RelayMetrics) SetProcessingTimestamp(timestamp time.Time) {
  $$$
}'

Length of output: 492


333-333: Log errors in AddMetricForProcessingLatencyAfterProvider.

Ensure that any errors in apil.logger.AddMetricForProcessingLatencyAfterProvider are logged.

protocol/chainlib/jsonRPC.go (1)

436-436: Add error handling for SetProcessingTimestamp.

Ensure that metricsData.SetProcessingTimestamp(startTime) is correctly executed without errors.

Verification successful

Add error handling for SetProcessingTimestamp.

Ensure that metricsData.SetProcessingTimestamp(startTime) is correctly executed without errors.

  • The SetProcessingTimestamp method in protocol/metrics/analytics.go includes a nil check to prevent errors.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that `SetProcessingTimestamp` is correctly executed without errors.

# Test: Search for the function definition. Expect: No errors in the implementation.
ast-grep --lang go --pattern $'func (rm *RelayMetrics) SetProcessingTimestamp(timestamp time.Time) {
  $$$
}'

Length of output: 492

protocol/chainlib/tendermintRPC.go (1)

588-590: Ensure AddMetricForProcessingLatencyAfterProvider runs after the response is sent.

The go routine for AddMetricForProcessingLatencyAfterProvider should be executed after sending the response to avoid delaying the response.

- go apil.logger.AddMetricForProcessingLatencyAfterProvider(metricsData, chainID, apiInterface)
return err
+ defer go apil.logger.AddMetricForProcessingLatencyAfterProvider(metricsData, chainID, apiInterface)
return err

Likely invalid or redundant comment.

protocol/rpcconsumer/rpcconsumer_server.go (7)

478-478: Verify correct usage of analytics parameter.

Ensure that the analytics parameter is correctly passed and utilized in sendRelayToProvider to log metrics accurately.


200-204: Verify correct usage of analytics parameter.

Ensure that the analytics parameter is correctly passed and utilized in sendRelayToProvider to log metrics accurately.

Verification successful

Verify correct usage of analytics parameter.

The analytics parameter is used in the initial relay attempts but not in retries. This appears to be an intentional design choice to avoid redundant logging during retries. However, please review the following locations to confirm this decision:

  • protocol/rpcconsumer/rpcconsumer_server.go lines where sendRelayToProvider is called without the analytics parameter.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify correct usage of `analytics` parameter in `sendRelayToProvider` calls.

# Test: Search for the function usage. Expect: All calls to `sendRelayToProvider` include `analytics` parameter.
rg --type go $'sendRelayToProvider'

Length of output: 1500


Script:

#!/bin/bash
# Description: Extract surrounding code for each call to `sendRelayToProvider` to verify the correct usage of the `analytics` parameter.

# Extract 5 lines before and after each call to `sendRelayToProvider`
rg --type go --context 5 'sendRelayToProvider' protocol/rpcconsumer/rpcconsumer_server.go

Length of output: 5768


691-691: Verify correct usage of analytics parameter.

Ensure that the analytics parameter is correctly passed and utilized in relayInner to log metrics accurately.

Verification successful

Verify correct usage of analytics parameter.

The analytics parameter is correctly passed and utilized within the relayInner function to log metrics accurately.

  • The analytics parameter is used to add metrics for processing latency before provider processing.
  • The analytics parameter is also used to set the processing timestamp after the provider replies.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify correct usage of `analytics` parameter in `relayInner` calls.

# Test: Search for the function usage. Expect: All calls to `relayInner` include `analytics` parameter.
rg --type go $'relayInner'

Length of output: 618


Script:

#!/bin/bash
# Extract the body of the relayInner function to verify the usage of the analytics parameter.
ast-grep --lang go --pattern $'func (rpccs *RPCConsumerServer) relayInner($_, $_, $_, $_, $_, $_, $_) ($_ , $_, $_) {
  $$$
}' --json

Length of output: 14289


811-813: Ensure accurate measurement of processing time before provider processing.

The logic to measure processing time before provider processing and log metrics should be verified for accuracy.

Verification successful

Ensure accurate measurement of processing time before provider processing.

The logic for measuring and logging the processing time before provider processing is accurate. The function AddMetricForProcessingLatencyBeforeProvider correctly measures the time difference between the current time and the processing timestamp, and this is done just before the provider processes the request.

  • protocol/metrics/rpcconsumerlogs.go (lines 157-161)
  • protocol/rpcconsumer/rpcconsumer_server.go (lines around the function call)
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify accurate measurement of processing time before provider processing.

# Test: Search for the function usage. Expect: All calls to `AddMetricForProcessingLatencyBeforeProvider` are accurate.
rg --type go $'AddMetricForProcessingLatencyBeforeProvider'

Length of output: 422


Script:

#!/bin/bash
# Retrieve the implementation of AddMetricForProcessingLatencyBeforeProvider
ast-grep --lang go --pattern $'func (rpccl *RPCConsumerLogs) AddMetricForProcessingLatencyBeforeProvider($_, $_, $_) {
  $$$
}' 

# Retrieve the relevant section of the code in rpcconsumer_server.go where AddMetricForProcessingLatencyBeforeProvider is called
rg --context 10 --type go 'AddMetricForProcessingLatencyBeforeProvider' protocol/rpcconsumer/rpcconsumer_server.go

Length of output: 2164


344-357: Verify correct usage of analytics parameter and processing timestamp logic.

Ensure that the analytics parameter is correctly passed and utilized in sendRelayToProvider and the processing timestamp is set accurately.


304-304: Verify correct usage of analytics parameter.

Ensure that the analytics parameter is correctly passed and utilized in ProcessRelaySend to log metrics accurately.


352-357: Verify correct usage of analytics parameter and processing timestamp logic.

Ensure that the analytics parameter is correctly passed and utilized in sendRelayToProvider and the processing timestamp is set accurately.

protocol/chainlib/tendermintRPC.go Outdated Show resolved Hide resolved
protocol/chainlib/tendermintRPC.go Outdated Show resolved Hide resolved
protocol/chainlib/tendermintRPC.go Outdated Show resolved Hide resolved
protocol/rpcconsumer/rpcconsumer_server.go Outdated Show resolved Hide resolved
protocol/chainlib/jsonRPC.go Outdated Show resolved Hide resolved
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 c896e97 and 3209970.

Files selected for processing (1)
  • protocol/rpcconsumer/rpcconsumer_server.go (10 hunks)
Additional comments not posted (5)
protocol/rpcconsumer/rpcconsumer_server.go (5)

200-204: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to sendRelayWithRetries correctly handle the PairingListEmptyError scenario.


304-304: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to SendRelay correctly handle the ProcessRelaySend function.


Line range hint 344-437:
LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to ProcessRelaySend correctly handle the PairingListEmptyError scenario.


478-478: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to sendRelayToProvider correctly pass the analytics parameter.


Line range hint 796-815:
LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to relayInner correctly handle the metrics logging.

protocol/metrics/metrics_consumer_manager.go Outdated Show resolved Hide resolved
protocol/metrics/metrics_consumer_manager.go Show resolved Hide resolved
protocol/metrics/rpcconsumerlogs.go Outdated Show resolved Hide resolved
protocol/metrics/rpcconsumerlogs.go Outdated Show resolved Hide resolved
protocol/rpcconsumer/rpcconsumer_server.go Outdated Show resolved Hide resolved
protocol/rpcconsumer/rpcconsumer_server.go Outdated Show resolved Hide resolved
protocol/chainlib/rest.go Outdated Show resolved Hide resolved
protocol/chainlib/rest.go Outdated Show resolved Hide resolved
protocol/chainlib/jsonRPC.go Outdated Show resolved Hide resolved
protocol/chainlib/grpc.go Outdated Show resolved Hide resolved
@oren-lava oren-lava requested a review from ranlavanet July 9, 2024 12:46
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 3209970 and a183f54.

Files selected for processing (7)
  • protocol/chainlib/grpc.go (2 hunks)
  • protocol/chainlib/jsonRPC.go (2 hunks)
  • protocol/chainlib/rest.go (4 hunks)
  • protocol/chainlib/tendermintRPC.go (4 hunks)
  • protocol/metrics/metrics_consumer_manager.go (6 hunks)
  • protocol/metrics/rpcconsumerlogs.go (1 hunks)
  • protocol/rpcconsumer/rpcconsumer_server.go (10 hunks)
Files skipped from review as they are similar to previous changes (7)
  • protocol/chainlib/grpc.go
  • protocol/chainlib/jsonRPC.go
  • protocol/chainlib/rest.go
  • protocol/chainlib/tendermintRPC.go
  • protocol/metrics/metrics_consumer_manager.go
  • protocol/metrics/rpcconsumerlogs.go
  • protocol/rpcconsumer/rpcconsumer_server.go

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 a183f54 and f0dc3a7.

Files selected for processing (2)
  • protocol/metrics/rpcconsumerlogs.go (1 hunks)
  • protocol/rpcconsumer/rpcconsumer_server.go (11 hunks)
Files skipped from review as they are similar to previous changes (2)
  • protocol/metrics/rpcconsumerlogs.go
  • protocol/rpcconsumer/rpcconsumer_server.go

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 f0dc3a7 and 9a956dc.

Files selected for processing (3)
  • protocol/metrics/analytics.go (2 hunks)
  • protocol/metrics/rpcconsumerlogs.go (1 hunks)
  • protocol/rpcconsumer/rpcconsumer_server.go (11 hunks)
Files skipped from review as they are similar to previous changes (3)
  • protocol/metrics/analytics.go
  • protocol/metrics/rpcconsumerlogs.go
  • protocol/rpcconsumer/rpcconsumer_server.go

protocol/metrics/rpcconsumerlogs.go Outdated Show resolved Hide resolved
protocol/rpcconsumer/rpcconsumer_server.go Outdated Show resolved Hide resolved
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

Outside diff range, codebase verification and nitpick comments (4)
protocol/chainlib/grpc.go (1)

333-333: Ensure errors in AddMetricForProcessingLatencyAfterProvider are logged.

The function AddMetricForProcessingLatencyAfterProvider does not handle errors within its implementation, and its usage in various files (protocol/chainlib/jsonRPC.go, protocol/chainlib/tendermintRPC.go, protocol/chainlib/grpc.go, protocol/chainlib/rest.go) does not include error handling or logging. To ensure robustness, it is recommended to add error handling and logging for this function.

  • protocol/metrics/rpcconsumerlogs.go (implementation)
  • protocol/chainlib/jsonRPC.go
  • protocol/chainlib/tendermintRPC.go
  • protocol/chainlib/grpc.go
  • protocol/chainlib/rest.go
Analysis chain

Log processing latency after provider.

The addition of apil.logger.AddMetricForProcessingLatencyAfterProvider(metricsData, apil.endpoint.ChainID, apiInterface) logs the processing latency after the relay operation. Ensure that any errors in this method are logged.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that errors in `AddMetricForProcessingLatencyAfterProvider` are logged.

# Test: Search for the function usage. Expect: Error handling/logging for the function.
rg --type go $'AddMetricForProcessingLatencyAfterProvider'

Length of output: 975


Script:

#!/bin/bash
# Extract the implementation of the `AddMetricForProcessingLatencyAfterProvider` function
ast-grep --lang go --pattern $'func (rpccl *RPCConsumerLogs) AddMetricForProcessingLatencyAfterProvider($_, $_, $_) {
  $$$
}' protocol/metrics/rpcconsumerlogs.go

# Check the context around the usage of `AddMetricForProcessingLatencyAfterProvider` in other files
rg -A 5 'AddMetricForProcessingLatencyAfterProvider' protocol/chainlib/jsonRPC.go protocol/chainlib/tendermintRPC.go protocol/chainlib/grpc.go protocol/chainlib/rest.go

Length of output: 3562

protocol/chainlib/jsonRPC.go (1)

510-513: Log processing latency after provider.

The addition of apil.logger.AddMetricForProcessingLatencyAfterProvider(metricsData, chainID, apiInterface) logs the processing latency after the relay operation. However, there is no error handling or logging for this method.

  • Ensure that any errors in AddMetricForProcessingLatencyAfterProvider are logged.
Analysis chain

Log processing latency after provider.

The addition of apil.logger.AddMetricForProcessingLatencyAfterProvider(metricsData, chainID, apiInterface) logs the processing latency after the relay operation. Ensure that any errors in this method are logged.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that errors in `AddMetricForProcessingLatencyAfterProvider` are logged.

# Test: Search for the function usage. Expect: Error handling/logging for the function.
rg --type go $'AddMetricForProcessingLatencyAfterProvider'

Length of output: 975


Script:

#!/bin/bash
# Description: Gather surrounding lines of code where `AddMetricForProcessingLatencyAfterProvider` is used to check for error handling and logging.
# Fetch 5 lines before and after each occurrence for context.

rg --type go -A 5 -B 5 'AddMetricForProcessingLatencyAfterProvider'

Length of output: 6151

protocol/chainlib/tendermintRPC.go (1)

519-521: Ensure errors in AddMetricForProcessingLatencyAfterProvider are logged.

The review comment is correct. The AddMetricForProcessingLatencyAfterProvider method calls do not have explicit error handling or logging. It is important to add error handling to log any potential errors from this method.

  • Files to update:
    • protocol/chainlib/tendermintRPC.go
    • protocol/chainlib/rest.go
    • protocol/chainlib/jsonRPC.go
    • protocol/chainlib/grpc.go
Analysis chain

Log processing latency after provider.

The addition of apil.logger.AddMetricForProcessingLatencyAfterProvider(metricsData, chainID, apiInterface) logs the processing latency after the relay operation. Ensure that any errors in this method are logged.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that errors in `AddMetricForProcessingLatencyAfterProvider` are logged.

# Test: Search for the function usage. Expect: Error handling/logging for the function.
rg --type go $'AddMetricForProcessingLatencyAfterProvider'

Length of output: 975


Script:

#!/bin/bash
# Description: Verify that errors in `AddMetricForProcessingLatencyAfterProvider` are logged.

# Search for the context around the `AddMetricForProcessingLatencyAfterProvider` method calls to check for error handling and logging.
rg --type go -A 5 -B 5 'AddMetricForProcessingLatencyAfterProvider'

Length of output: 6151

protocol/rpcconsumer/rpcconsumer_server.go (1)

477-477: Ensure all calls to sendRelayToProvider match the new function signature.

The following calls to sendRelayToProvider need to be updated to include the analytics parameter:

  • Line 477: err = rpccs.sendRelayToProvider(ctx, chainMessage, relay, "-init-", "", relayProcessor, nil)
  • Line 481: err = rpccs.sendRelayToProvider(ctx, chainMessage, relay, "-init-", "", relayProcessor, nil)
  • Line 486: err = rpccs.sendRelayToProvider(ctx, chainMessage, relayRequestData, dappID, consumerIp, relayProcessor, nil)
  • Line 491: err = rpccs.sendRelayToProvider(ctx, chainMessage, relayRequestData, dappID, consumerIp, relayProcessor, nil)
  • Line 496: err = rpccs.sendRelayToProvider(ctx, chainMessage, relayRequestData, dappID, consumerIp, relayProcessor, nil)

Please ensure these calls are updated to include the analytics parameter if applicable.

Analysis chain

LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to sendRelayToProvider match the new signature.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `sendRelayToProvider` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'sendRelayToProvider'

Length of output: 5286

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9a956dc and 0c492ab.

Files selected for processing (7)
  • protocol/chainlib/grpc.go (2 hunks)
  • protocol/chainlib/jsonRPC.go (2 hunks)
  • protocol/chainlib/rest.go (4 hunks)
  • protocol/chainlib/tendermintRPC.go (4 hunks)
  • protocol/metrics/analytics.go (2 hunks)
  • protocol/metrics/rpcconsumerlogs.go (1 hunks)
  • protocol/rpcconsumer/rpcconsumer_server.go (11 hunks)
Additional comments not posted (15)
protocol/metrics/analytics.go (3)

53-58: LGTM!

The method SetProcessingTimestampBeforeRelay correctly sets the ProcessingTimestamp field.


60-67: LGTM!

The method SetProcessingTimestampAfterRelay correctly sets the ProcessingTimestamp field and updates the MeasureAfterProviderProcessingTime flag.


16-27: LGTM! But verify the usage of new fields.

The new fields ProcessingTimestamp and MeasureAfterProviderProcessingTime are well-integrated within the RelayMetrics struct.

However, ensure that all instances where these fields are used are correctly updated in the codebase.

Verification successful

Verified: The new fields ProcessingTimestamp and MeasureAfterProviderProcessingTime are correctly used in the codebase.

The fields are consistently set and checked across multiple files, ensuring proper integration.

  • protocol/rpcconsumer/rpcconsumer_server.go
  • protocol/metrics/rpcconsumerlogs.go
  • protocol/metrics/analytics.go
  • protocol/chainlib/rest.go
  • protocol/chainlib/jsonRPC.go
  • protocol/chainlib/tendermintRPC.go
  • protocol/chainlib/grpc.go
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of new fields in the codebase.

# Test: Search for the usage of `ProcessingTimestamp` and `MeasureAfterProviderProcessingTime`. Expect: Correct usage in all instances.
rg --type go 'ProcessingTimestamp|MeasureAfterProviderProcessingTime'

Length of output: 2059

protocol/metrics/rpcconsumerlogs.go (2)

159-163: LGTM!

The method AddMetricForProcessingLatencyBeforeProvider correctly calculates and records the processing latency before a relay is sent to a provider.


165-169: LGTM!

The method AddMetricForProcessingLatencyAfterProvider correctly calculates and records the processing latency after a relay is received from a provider.

protocol/chainlib/rest.go (1)

311-311: LGTM! But verify the usage of new methods.

The changes to the Serve method are well-integrated and enhance the tracking of processing latency before and after provider interactions.

However, ensure that all instances where the new methods SetProcessingTimestampBeforeRelay and AddMetricForProcessingLatencyAfterProvider are used are correctly updated in the codebase.

Also applies to: 346-349, 364-364, 421-423

protocol/chainlib/grpc.go (1)

321-321: Set processing timestamp before relay.

The addition of metricsData.SetProcessingTimestampBeforeRelay(startTime) captures the processing timestamp before the relay operation. Ensure that startTime is correctly set and used.

protocol/chainlib/jsonRPC.go (1)

436-436: Set processing timestamp before relay.

The addition of metricsData.SetProcessingTimestampBeforeRelay(startTime) captures the processing timestamp before the relay operation. Ensure that startTime is correctly set and used.

protocol/chainlib/tendermintRPC.go (3)

458-458: Set processing timestamp before relay.

The addition of metricsData.SetProcessingTimestampBeforeRelay(startTime) captures the processing timestamp before the relay operation. Ensure that startTime is correctly set and used.


539-539: Set processing timestamp before relay.

The addition of metricsData.SetProcessingTimestampBeforeRelay(startTime) captures the processing timestamp before the relay operation. Ensure that startTime is correctly set and used.


588-590: Log processing latency after provider.

The addition of apil.logger.AddMetricForProcessingLatencyAfterProvider(metricsData, chainID, apiInterface) logs the processing latency after the relay operation. Ensure that any errors in this method are logged.

protocol/rpcconsumer/rpcconsumer_server.go (4)

200-204: Verify the function usage of sendRelayToProvider.

The sendRelayToProvider function now includes the analytics parameter. Ensure that all calls to this function correctly handle this new parameter.

Verification successful

Verified the function usage of sendRelayToProvider.

All calls to the sendRelayToProvider function correctly handle the new analytics parameter where appropriate.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `sendRelayToProvider` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'sendRelayToProvider'

Length of output: 5286


344-357: LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to ProcessRelaySend match the new signature.


304-304: Verify the function usage of ProcessRelaySend.

The ProcessRelaySend function now includes the analytics parameter. Ensure that all calls to this function correctly handle this new parameter.

Verification successful

Verified the function usage of ProcessRelaySend.

All calls to ProcessRelaySend correctly handle the new analytics parameter.

  • protocol/rpcconsumer/rpcconsumer_server.go (lines 304 and function definition)
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `ProcessRelaySend` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'ProcessRelaySend'

Length of output: 1691


Script:

#!/bin/bash
# Description: Verify all function calls to `ProcessRelaySend` match the new signature.

# Test: Search for all occurrences of the function usage. Expect: Only occurrences of the new signature.
rg 'ProcessRelaySend\(' --type go

Length of output: 539


Line range hint 795-812:
LGTM! But verify the function usage in the codebase.

The code changes are approved.

However, ensure that all function calls to relayInner match the new signature.

Verification successful

LGTM!

The function relayInner is correctly used with the new signature in the codebase.

  • protocol/rpcconsumer/rpcconsumer_server.go:
    • Line 806: func (rpccs *RPCConsumerServer) relayInner(ctx context.Context, singleConsumerSession *lavasession.SingleConsumerSession, relayResult *common.RelayResult, relayTimeout time.Duration, chainMessage chainlib.ChainMessage, consumerToken string, analytics *metrics.RelayMetrics)
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `relayInner` match the new signature.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type go -A 5 $'relayInner'

Length of output: 1592


Script:

#!/bin/bash
# Description: Verify all function calls to `relayInner` match the new signature across the entire codebase.

# Test: Search for the function usage in the entire codebase. Expect: Only occurrences of the new signature.
rg --type go 'relayInner'

Length of output: 618

@ranlavanet ranlavanet merged commit f322a0f into main Jul 23, 2024
30 checks passed
@ranlavanet ranlavanet deleted the consumer-time-overhead-metrics branch July 23, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants