feat(telemetry): track local payments #2610
Closed
Conversation
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
* ci: WIP new integration test workflow no idea what the docker command will do... * fix: deps not found, add debug step * ci: add debug step * ci: use docker compose (v2) instead of docker-compose (v1) * chore: add setup from seed debug logs * ci: rework jobs into steps * chore: rm todo comment * chore: rm debug log * chore: shorten timeout in integration test * refactor: move integreation test job to lint_test_build * chore: test hostile cmd in ci, no setup host step * Revert "chore: test hostile cmd in ci, no setup host step" This reverts commit 5f529b2.
beniaminmunteanu
requested review from
mkurapov,
sabineschaller and
JoblersTune
github-actions
bot
added
type: tests
✅ Deploy Preview for brilliant-pasca-3e80ec ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
mkurapov
reviewed
Mar 26, 2024
packages/backend/src/payment-method/ilp/connector/core/middleware/telemetry.ts
Outdated
Show resolved
Hide resolved
|
Based on these changes, @mkurapov also brought up having 4 metrics, 2 for local and 2 for remote. |
mkurapov
reviewed
Mar 27, 2024
packages/backend/src/payment-method/ilp/connector/core/test/middleware/telemetry.test.ts
Outdated
Show resolved
Hide resolved
packages/backend/src/payment-method/ilp/connector/core/test/middleware/telemetry.test.ts
Show resolved
Hide resolved
mkurapov
reviewed
Mar 28, 2024
packages/backend/src/payment-method/ilp/connector/core/test/middleware/telemetry.test.ts
Outdated
Show resolved
Hide resolved
packages/backend/src/payment-method/ilp/connector/core/test/middleware/telemetry.test.ts
Outdated
Show resolved
Hide resolved
packages/backend/src/payment-method/ilp/connector/core/test/middleware/telemetry.test.ts
Outdated
Show resolved
Hide resolved
mkurapov
approved these changes
Apr 2, 2024
| const collectAmountSpy = jest | ||
| .spyOn(telemetry, 'collectTelemetryAmount') | ||
| .mockImplementation(() => { | ||
| expect(nextCalled).toBe(true) |
There was a problem hiding this comment.
Ah, I understand, this ensures that next was called before the collectTelemetryAmount 👍
I was looking at some more of this, and this is something that exists in jest-extended, but not in the standard jest lib unfourtunately.
We can keep this test as is.
beniaminmunteanu
force-pushed
the
2531-telemetry-local-payments
branch
from
7095b15 to
566e1be
Compare
|
closed in favor of #2626 (which is a fresh duplicate). Will delete this source branch once that one gets merged |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Changes proposed in this pull request
This PR moves the amount metrics collection from the accounting service back to the ILP connector, as an extra middleware.
Context
All context is provided in the linked issue
fixes #2531 #2505
Checklist
Note
if you enable telemetry on the playground containers, you should be able to see the metrics output of your transactions here