-
Notifications
You must be signed in to change notification settings - Fork 89
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: added packet count telemetry metrics #2797
Merged
Merged
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
6e29243
feat: added packet count and amount metrics to the connector core
JoblersTune 720a702
chore: replaced a line in the README that is true again
JoblersTune 38c82d1
chore: updated README from feedback
JoblersTune 24c5dbf
chore: formatting
JoblersTune 023d643
chore: made outgoing payment tests more specific
JoblersTune 3bc58d7
Apply suggestions from code review
JoblersTune b958b34
Update packages/backend/src/payment-method/ilp/connector/core/rafiki.ts
JoblersTune 17852af
test: added non-positive cases
JoblersTune f5b5bf4
Merge branch '2734/sj/telemetry-count-packets' of github.com:interled…
JoblersTune cffe639
chore: formatting
JoblersTune 1c29f9d
docs: removed docs changes due to code freeze
JoblersTune File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
packages/backend/src/payment-method/ilp/connector/core/README.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,47 @@ | ||
# `core` | ||
|
||
> Toolkit comprised of core services and middleware needed to develop ILP applications. | ||
|
||
## Telemetry Collection | ||
|
||
Currently, we collect packet count and packet amount metrics directly within the connector core. These metrics are captured at the Interledger layer to track packet activity while ensuring privacy by collecting amounts at the packet level, rather than at the transaction level. This approach helps to preserve privacy, as we do not expose entire transaction amounts, while also incorporating privacy-preserving measures into the collected amounts. You can read more about privacy [here](https://rafiki.dev/telemetry/privacy/). | ||
|
||
### Why We Collect on the Sending Side | ||
|
||
The first decision in collecting this data was whether to do so on the sender's side or the receiver's side. We opted for the sender’s side to maintain consistency across our metrics, particularly for calculating average transaction amounts and times, which are tied to the outgoing payment flow in the Open Payments (OP) layer. By collecting metrics from the same perspective, we ensure alignment with how other metrics (like transaction completion times) are captured. | ||
|
||
We also considered collecting metrics on both the sending and receiving sides, capturing metrics when prepare packets were received by the receiver and when fulfill or reject responses were received by the sender. However, this could lead to unreliable metrics, as telemetry is optional and might not be enabled on all nodes. | ||
|
||
### Why We Chose to collect metrics before and after the sendering node's middlware routes | ||
JoblersTune marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Given these considerations, we decided to place our packet count and amount metrics within the Rafiki Connector Core. The core plays a crucial role in processing ILP packets, handling both outgoing payments and quotes. | ||
|
||
The specific metrics we collect here include: | ||
|
||
- packet_count_prepare: Counts the prepare packets sent, collected before the middleware routes are executed. | ||
- packet_count_fulfill: Counts the fulfill packets received, collected after receiving a reply from the receiver. | ||
- packet_count_reject: Counts the reject packets received. | ||
- packet_amount_fulfill: Records the amount sent in fulfill packets. | ||
|
||
These metrics provide valuable insights, including our success and error rates when sending packets over the network. | ||
|
||
### Challenges with the Current Setup | ||
|
||
Sender-Side Limitation: Currently, metrics are only collected on the sender side. This is adequate for now but does not capture data from connecting nodes, which we plan to address in future implementations when multi-hop support is added. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be good to add a short explanation for the connector nodes: eg in A > B > C, we only collect packet information in A |
||
|
||
In a scenario where a sender (node A) wants to send money to the receiver (node C), and this transaction takes place via an intermediray (node B). | ||
|
||
sender (node A) --> connector (node B) --> receiver (node C) | ||
|
||
With the cuirrent metric collection location, we only collect packet count and amount infomation from the sender (at node A) and we miss the packets and amounts forwarded by the connecting node (node B). Ideally we'd like to collect information from the sender and the connector nodes once multihop routing has been implemented. | ||
JoblersTune marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### Other Considered Locations for Metrics Collection | ||
|
||
We explored several alternative locations for collecting telemetry metrics within the Rafiki Connector Core: | ||
|
||
- Dedicated Middleware: Initially, we implemented a dedicated telemetry middleware. However, this approach resulted in data being duplicated on both the sender and receiver sides, leading to inaccurate metrics. To address this, we would need to filter the data to ensure metrics are only collected on the sender side. Additionally, the telemetry middleware would need to effectively handle errors thrown in the middleware chain. To achieve this, it might be necessary to place the telemetry middleware right before the error-handling middleware, allowing it to catch and reflect any errors that occur. This would involve collecting the prepare packet count, wrapping the `next()` function in a try-catch block to capture any new errors, and then collecting reply and amount metrics after `next()` has resolved. | ||
- Around the ILP packet middleware on the Receiving Side: We also considered adding telemetry around the middleware on receiving and connecting nodes. However, this would lead to a fragmented metric collection, with some metrics gathered on the receiver side and others on the sender side. This fragmentation could complicate the handling of concepts like transaction count, which would require dual collection on both sender and receiver sides. We'd also have to watch for the possibility of data duplication on the connectors because their middleware might trigger in each direction, as they receive prepares and again as they receive responses. | ||
|
||
### Moving Forward | ||
|
||
As we implement multi-hop capabilities and further refine the connector architecture, we will likely identify better entry and exit points for telemetry collection. This will allow us to capture a more comprehensive set of metrics across both sender and receiver nodes, ensuring a complete and accurate understanding of the network’s behavior. |
30 changes: 0 additions & 30 deletions
30
packages/backend/src/payment-method/ilp/connector/core/middleware/telemetry.ts
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
59 changes: 59 additions & 0 deletions
59
packages/backend/src/payment-method/ilp/connector/core/telemetry.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
import { TelemetryService } from '../../../../telemetry/service' | ||
import { IlpResponse } from './middleware/ilp-packet' | ||
import { ValueType } from '@opentelemetry/api' | ||
|
||
export function incrementPreparePacketCount( | ||
unfulfillable: boolean, | ||
prepareAmount: string, | ||
JoblersTune marked this conversation as resolved.
Show resolved
Hide resolved
|
||
telemetry: TelemetryService | ||
): void { | ||
if (!unfulfillable && Number(prepareAmount)) { | ||
telemetry.incrementCounter('packet_count_prepare', 1, { | ||
description: 'Count of prepare packets that are sent' | ||
}) | ||
} | ||
} | ||
|
||
export function incrementFulfillOrRejectPacketCount( | ||
unfulfillable: boolean, | ||
prepareAmount: string, | ||
JoblersTune marked this conversation as resolved.
Show resolved
Hide resolved
|
||
response: IlpResponse, | ||
telemetry: TelemetryService | ||
): void { | ||
if (!unfulfillable && Number(prepareAmount)) { | ||
if (response.fulfill) { | ||
telemetry.incrementCounter('packet_count_fulfill', 1, { | ||
description: 'Count of fulfill packets' | ||
}) | ||
} else if (response.reject) { | ||
telemetry.incrementCounter('packet_count_reject', 1, { | ||
description: 'Count of reject packets' | ||
}) | ||
} | ||
} | ||
} | ||
|
||
export async function incrementAmount( | ||
unfulfillable: boolean, | ||
prepareAmount: string, | ||
response: IlpResponse, | ||
code: string, | ||
scale: number, | ||
telemetry: TelemetryService | ||
): Promise<void> { | ||
if (!unfulfillable && Number(prepareAmount) && response.fulfill) { | ||
const value = BigInt(prepareAmount) | ||
await telemetry.incrementCounterWithTransactionAmount( | ||
'packet_amount_fulfill', | ||
{ | ||
value, | ||
assetCode: code, | ||
assetScale: scale | ||
}, | ||
{ | ||
description: 'Amount sent through the network', | ||
valueType: ValueType.DOUBLE | ||
} | ||
) | ||
} | ||
} |
113 changes: 0 additions & 113 deletions
113
packages/backend/src/payment-method/ilp/connector/core/test/middleware/telemetry.test.ts
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding this