-
Notifications
You must be signed in to change notification settings - Fork 90
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(localenv): span metrics generation #2849
Conversation
- adds configuration that generates span metrics from tempo traces - can see new `traces_spanmetrics_bucket` etc. in local grafana dashboard
✅ Deploy Preview for brilliant-pasca-3e80ec canceled.
|
I considered if we wanted to add visualizations for each resolver. Like stat metrics for 25th, 50th, 95th percentile etc or the heatmap/histogram like we have for the pay times but opted not to. First, I feel like we will better understand what details we need as we actually consume these (as part of performance testing analysis?). Second, I think we probably mostly care about the extreme high end (ie 95th, 99th percentile etc). In which case maybe we just add another bar gauge like the included one but for 99th percentile. Open to other ideas for what visualizations we need for this but I think this one gives us the gist of what we're looking for. |
I'm curious exactly what our plan is with the local dashboard? Are we using it for dev? Are we using it to measure only certain local metrics. It's not exactly clear to me. |
"refId": "A" | ||
} | ||
], | ||
"title": "Panel Title", |
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.
lets update this title
"uid": "PBFA97CFB590B2093" | ||
}, | ||
"editorMode": "code", | ||
"expr": "histogram_quantile(0.95, sum(rate(traces_spanmetrics_latency_bucket{span_name=~\"^(mutation|query).*\"}[$__rate_interval])) by (le, span_name))", |
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.
should this be something other than$__rate_interval
, but instead the selected interval of the dashboard? That way you can see the timings per last x minutes/seconds etc
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.
should this be something other than$__rate_interval, but instead the selected interval of the dashboard?
From what I can tell it does factor in the current time range. I spun up the localenv, ran some queries and saw the data in this visualization with 5m time range. I waited 5m+ and saw no data until I bumped to 15m time range.
Im also seeing it generally recommended as starting point for the rate arg :
Mainly for performance testing and debugging |
I'm mostly using it to validate the metric collection and develop visualizations. If it were applicable to the live version I would add them there after merging this (although I guess technically it wouldn't have any data until the next release). I dont think we need to maintain parity with the live version or have examples for every single metric, but its nice to have some basic proof-of-concept visualizations for the different types of metrics (traces, histograms, counts, etc.) IMO. Thinking back to our conversation about development workflow I think in theory it would be nice to develop locally, commit, then publishing to grafana from ci. This would unify it with our general change workflow and it would be version controlled. But not sure its worth the setup tbh. |
* feat(localenv): add span metric generation - adds configuration that generates span metrics from tempo traces - can see new `traces_spanmetrics_bucket` etc. in local grafana dashboard * feat(localenv): add gql resolver metric * chore(localenv): give panel title
* feat(backend): make keys unique * fix: only make keys unique per wallet address * fix(frontend): It is ambiguous on what scale is the withdrawal and deposit input (#2817) * fix(frontend): asset scale consistency in liquidity dialogs. * Ensure asset scale consistency when displaying and withdrawing liquidity by adding asset info to the liquidity dialog component and updating the input handling in Rafiki Admin UI. --------- Co-authored-by: Blair Currey <12960453+BlairCurrey@users.noreply.github.com> * chore: sync docs and readmes (#2830) * fix: getting the localenv docs and readme in sync * chore: updated MASE screenshots * chore: updating the code block language identifier to have consistent approach through the docs * chore(deps): update dependency @apollo/client to ^3.11.2 (#2822) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * feat(frontend): ux improvements to liquidity dialog component (#2839) * fixed(frontend) asset page now retains page scroll position. * feat(frontend) added autofocus to liquidity dialog input * feat(fronted) made eslint happy * feat(docker): switch to alpine3.19 (#2842) * feat(auth): build with alpine3.19 * feat(backend): build with alpine3.19 * feat(frontend): build with alpine3.19 * bump(localenv): docker image to alpine 3.19 * fix(auth): interact redirect (#2832) * fix(auth): interact redirect * fix(auth): session cookie not expiring in browser * fix(auth): session expiration time unit --------- Co-authored-by: Blair Currey <12960453+BlairCurrey@users.noreply.github.com> * feat(interaction): return grantId (#2843) * feat(auth): return granId for the grant lookup via interaction id * test(auth): check grantId is returned for grant lookup via interaction id * docs(openapi): auth return grantId for grant lookup via interaction id * feat(outgoing-payment): add grantId to admin api (#2841) * feat(backend): support for returning grantId when querying outgoing payment When querying outgoing payment, either single one, or list of them via pagination, etc., it will be possible to also get a grantId under which the outgoing * test(outgoing-payment): check if grantId is returned * docs(bruno): include grantId when fetching outgoing payment * feat(auth): soft delete access tokens and grant accesses (#2837) * feat(auth): set session expiry based on interaction expiry env (#2851) * feat(localenv): span metrics generation (#2849) * feat(localenv): add span metric generation - adds configuration that generates span metrics from tempo traces - can see new `traces_spanmetrics_bucket` etc. in local grafana dashboard * feat(localenv): add gql resolver metric * chore(localenv): give panel title * chore(deps): update dependency @types/node to ^20.14.15 (#2838) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update dependency @apollo/client to ^3.11.4 (#2845) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * feat(2737): add fees as metric for outgoing payment. (#2831) * feat(2737): add fees as metric for outgoing payment. * feat(2737): rename to payment_fees. * feat(2737): test case updates. * feat(2737): formatting. * feat(2737): re-order test cases. Move fee collector. * feat(2737): dashboard and doc updates. * feat(2737): merged with main. * feat(2737): review feedback applied from @JoblersTune. * feat(2737): review feedback applied from @mkurapov. * feat(2737): additional tests for covert of assets and rates. * feat(2737): additional tests ensuring the increment counter was called. * feat(2737): additional tests ensuring the increment counter was called. * feat(2737): readme. * refactor(dependencies): axios to 1.7.4 (#2861) Our builds are failing due to Trivy scanner. Trivy scanner actually found that our Axios version v1.6.8 has a vulnerability - CVE-2024-39338. This was fixed in version 1.7.4, hence, the upgrade. fix #2860 * chore: add tests and better error handling * chore: formatting * fix: build * fix: add camelcase quotes and make `up` async * chore: keep latest version of key * fix: formatting * Added unrevoke wallet address key query resolver * Updated migration and removed unrevoked resolver * Checkstyle fix * Updated walletAddressKey deletion migration, removed unnecessary test for walletAddressKey service * Use ctid instead of createdAt to determine which rows are deleted * Fixed typo * Delete the least recent rows that have the same kid and are unrevoked * Updated delete script * Added revoked false back * Delete only the active keys --------- Co-authored-by: Emanuel Palestino <75344407+Emanuel-Palestino@users.noreply.github.com> Co-authored-by: Blair Currey <12960453+BlairCurrey@users.noreply.github.com> Co-authored-by: Sarah Jones <sarah38186@gmail.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Tadej Golobic <tadej@interledger.org> Co-authored-by: Nathan Lie <lie4nathan@gmail.com> Co-authored-by: Jason Bruwer <koekiebox@users.noreply.github.com> Co-authored-by: Oana Lolea <oana.lolea@breakpointit.eu>
Changes proposed in this pull request
Visualization preview:
Context
fixes: #2848
Checklist
fixes #number