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(backend): add trace to outgoing payment lifecycle #2884

Merged
merged 3 commits into from
Aug 27, 2024

Conversation

njlie
Copy link
Contributor

@njlie njlie commented Aug 22, 2024

Changes proposed in this pull request

  • Wraps the handlePaymentLifecycle function in a trace that can be consumed by Rafiki's telemetry services.

Context

Closes #2876.

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Bruno collection updated

@github-actions github-actions bot added pkg: backend Changes in the backend package. type: source Changes business logic labels Aug 22, 2024
Copy link

netlify bot commented Aug 22, 2024

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit dfe6b1d
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/66cd8be6791d7500086460aa

@njlie
Copy link
Contributor Author

njlie commented Aug 22, 2024

Regarding this statement from the issue:

A consideration: because telemetry is optional, you might need to create the traceProvider outside of if (Config.enableTelemetryTraces) conditional

OpenTelemetry handles it like so:

If a TracerProvider is not created, the OpenTelemetry APIs for tracing will use a no-op implementation and fail to generate data.

Handling the outgoing payment lifecycle without the enableTelemetryTraces env variable will still run successfully.

@@ -74,6 +74,7 @@ services:
WALLET_ADDRESS_URL: ${CLOUD_NINE_WALLET_ADDRESS_URL:-https://cloud-nine-wallet-backend/.well-known/pay}
ILP_CONNECTOR_URL: ${CLOUD_NINE_CONNECTOR_URL:-http://cloud-nine-wallet-backend:3002}
ENABLE_TELEMETRY: true
ENABLE_TELEMETRY_TRACES: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I think since we have this in the telemetry docker compose we can keep it turned off here IMO

deps.logger.warn('unexpected payment in lifecycle')
return
}
const tracer = trace.getTracer('outgoing_payment_worker')
Copy link
Contributor

Choose a reason for hiding this comment

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

actually, let us also include the code where we actually fetch the outgoing payment from the DB (it's happening in getPendingPayment)

@mkurapov mkurapov force-pushed the nl/2876/outgoing-payment-worker-trace branch from 5a09d45 to 54119bd Compare August 27, 2024 08:17
@mkurapov mkurapov force-pushed the nl/2876/outgoing-payment-worker-trace branch from 54119bd to dfe6b1d Compare August 27, 2024 08:18
@mkurapov mkurapov merged commit b9be601 into main Aug 27, 2024
42 checks passed
@mkurapov mkurapov deleted the nl/2876/outgoing-payment-worker-trace branch August 27, 2024 09:09
golobitch added a commit that referenced this pull request Sep 2, 2024
* refactor(backend): update rate caching (#2891)

* chore(localenv): fix startup migration (#2897)

* feat(backend): add trace to outgoing payment lifecycle (#2884)

* feat(backend): add trace to outgoing payment lifecycle

* feat(backend): encapsulate query lookup in trace

* chore(localenv): remove traces from default localenv

---------

Co-authored-by: Max Kurapov <[email protected]>

* Add migration files

* Update migration files

* Fix file name for migrations

* feat(tenant): basic tenant admin api schema and service

* Remove cascade deletion

* feat(auth): create basic tenant service and model plus graphql schema

* feat(graphql): generated data

* feat(graphql): add create tenant resolver and call service and update graphql schema

* feat(auth): add basic tenant schema and appropriate resources like model and service

* feat(generated): graphql schema

* feat(auth): add tenant id to create tenant input

* feat(auth): rename tenant id and add basic logic for calling create tenant service

* feat(auth): add basic create tenant functionality in service

* Add tenant model in backend

* feat(backend): add apollo client do dependencies

* feat(auth): add delete tenant mutation to the schema

* feat(auth): delete tenant

* Add tenantId field on resources models, update migration

* chore(auth): format

* feat(backend): create tenant service implementation

* feat(auth): codegen for putting generated files to backend

* feat(packages): make multi tenant work wip

* feat(mock-lib): add tenants to the seeding step

* feat(backend): update resolvers with tenant id and finish the tenant creation

* feat(localenv): update seed and docker compose

* feat(generated): graphql schema

* feat(bruno): admin auth create tenant mutation

* feat(backend): small changes to schema + mapping of tenant to graphql + bruno

* feat(everything): move endpoints to separate service, update bruno and schema do pagination and stuff

---------

Co-authored-by: Max Kurapov <[email protected]>
Co-authored-by: Nathan Lie <[email protected]>
Co-authored-by: bsanduc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: backend Changes in the backend package. type: source Changes business logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add trace to outgoing payment worker
3 participants