feat(metrics): add metrics plugin poc - #145
Conversation
|
🤖 Finished Review · ✅ Success · Started 10:15 AM UTC · Completed 10:36 AM UTC Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #145 +/- ##
===========================================
- Coverage 40.75% 18.61% -22.14%
===========================================
Files 147 147
Lines 10152 10152
===========================================
- Hits 4137 1890 -2247
- Misses 5911 8236 +2325
+ Partials 104 26 -78
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
ReviewFindingsHigh
Medium
Low
Labels: New metrics plugin feature with a high-severity missing-authentication finding requiring human security judgment. Next steps:
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the review comment for full details.
Note: The following inline comments could not be posted on the diff (GitHub returned 422) and are included here instead:
backend/plugins/metrics/cmd/main.go:48: [high] Missing-Authentication
The entire metrics API is exposed without any authentication or authorization. All endpoints are publicly accessible. The API reads from DevLake MySQL containing repository metadata, PR titles, descriptions, author names, PR URLs, and review comments.
Suggested fix: Add an authentication mechanism (API key via header, OAuth2, or mutual TLS). At minimum, require a shared secret via environment variable validated in middleware before any handler executes.
backend/plugins/metrics/api/routes/pr/handlers/key_metrics.go:41: [medium] logic-error
The key-metrics handler silently continues and returns partial data when the outliers query fails. The error is logged but execution continues to WriteJSON, producing a response with a valid-looking outlier_prs count from stats but an empty drill-down list.
Suggested fix: Either return an error for the outlier query failure or set the Warning field on the response.
backend/plugins/metrics/transform/pr/zscore.go:63: [medium] edge-case
When computeBaseline returns no valid lookback PRs, it returns muLog=0, sigmaLog=1e-10. The z-score calculation produces extremely large values, classifying ALL current-period PRs as Slow when there is no lookback data.
Suggested fix: When computeBaseline returns fallback values, return an empty/warning response or categorize all PRs as Average.
backend/plugins/metrics/Dockerfile:1: [medium] Dockerfile-correctness
Dockerfile uses golang:1.26-alpine as the builder image. Go 1.26 does not exist. This will cause a build failure when the image is pulled.
Suggested fix: Change the base image to match the Go version in go.mod (e.g., golang:1.21-alpine or golang:1.22-alpine).
backend/plugins/metrics/api/params.go:47: [medium] unbounded-request-body
DecodeQueryParams calls json.NewDecoder(r.Body).Decode without limiting body size. An attacker can send a multi-gigabyte POST body to exhaust server memory (DoS).
Suggested fix: Wrap r.Body with http.MaxBytesReader before decoding.
backend/plugins/metrics/api/routes/pr/handlers/utils.go:29: [medium] no-input-validation
toParams passes user-supplied values to SQL query parameters without validation. While parameterized queries prevent SQL injection, no validation on BlueprintID format, timestamp ranges, or array lengths means excessively large arrays generate long IN clauses causing query parsing overhead.
Suggested fix: Validate BlueprintID matches ^[0-9,]+$, From > 0, To > From, cap array lengths.
backend/plugins/metrics/api/routes/pr/handlers/key_metrics.go:33: [low] edge-case
Previous-period calculation creates a 1-second gap between periods. The From>0 guard prevents the degenerate case.
backend/plugins/metrics/transform/pr/zscore.go:150: [low] population-vs-sample-variance
computeBaseline uses population variance (divides by N) instead of sample variance (N-1). Impact is negligible for typical 90-day lookback but matters for small N.
backend/plugins/metrics/api/server.go:38: [low] race-condition
WriteJSON writes Content-Type header then encodes. If Encode fails partway, client receives partial 200 response. Standard Go HTTP limitation.
backend/plugins/metrics/transform/pr/zscore_test.go:48: [low] test-inadequate
TestBuildZScore_Categories uses single-PR lookback (degenerate sigma=1e-10), asserting total==2 without verifying category assignments.
backend/plugins/metrics/api/middleware.go:27: [low] CORS-fail-open
CORS origin is configurable via env var. If set to *, any browser origin can make requests. Default (https://devtools.pages.redhat.com) is safe.
backend/plugins/metrics/api/server.go:40: [low] information-disclosure
Healthz returns raw MySQL error messages to clients, potentially leaking connection details.
backend/plugins/metrics/cmd/main.go:57: [low] credential-handling
Default MySQL credentials could cause partial misconfiguration. MYSQL_PASS is required, mitigating the worst case.
backend/plugins/metrics/cmd/main.go:34: [low] no-rate-limiting
No rate limiting. The 10-connection DB pool provides implicit limiting, but explicit rate limiting is recommended before production use.
docs/upstream-diffs.md(file-level): Line 6 · [low] stale-enumeration
Owned plugins list does not include the new metrics plugin.
fullsend-ai-review
Bot
added
enhancement
mfrancisc
left a comment
mfrancisc
left a comment
There was a problem hiding this comment.
Great Starting !
Overall the approach looks good, I think we could explore making this a "real" plugin, similar to the aireview one, as already mentioned in the PR comment.
| @@ -0,0 +1,245 @@ | |||
| # Metrics API | |||
|
|
|||
| Standalone HTTP API that queries DevLake’s MySQL and returns pre-computed PR metrics for the developer dashboards. Lives under `backend/plugins/metrics/` as an independent binary (not registered in the DevLake plugin framework). | |||
There was a problem hiding this comment.
Instead of just having the code in the plugins folder but without it being registered and ran as a plugin,
we could develop a real plugin that doesn't have collector and pipeline subtasks.
A plugin can be API-only (PluginMeta + PluginApi), take a look at aireview plugin on how it does it.
Those handlers run in-process, use DevLake’s DAL/MySQL pool, and sit behind the same OIDC / API-key / CSRF stack in CreateApiServer(). So basically we also gain auth out of the box and other benefits.
| WHERE FIND_IN_SET(bp.id, ?) | ||
| AND r.name IN %s | ||
| AND a.user_name IS NOT NULL AND a.user_name != '' | ||
| AND ( | ||
| (UNIX_TIMESTAMP(pr.closed_date) BETWEEN ? AND ?) | ||
| OR (UNIX_TIMESTAMP(pr.created_date) BETWEEN ? AND ?) | ||
| ) |
There was a problem hiding this comment.
I think we will need to exclude also comments that belong to the PR author ?
Something like a.user_name != bp.author_name ?
| outliers, err := qpr.KeyMetricsOutliers(r.Context(), db, params) | ||
| if err != nil { | ||
| log.Printf("pr/key-metrics: outliers: %v", err) | ||
| } |
There was a problem hiding this comment.
I think here If KeyMetricsOutliers fails, the handler logs and still returns 200 with stats (including a non-zero outlier count) and an empty drill-down list.
3 participants
Summary
Currently, we are facing a few challenges while maintaining n8n workflows:
All together can lead to bugs, since we are not keeping track of all the changes and we are not testing them. As an alternative, we could create a Standalone HTTP API, written in Go, that queries DevLake’s MySQL database and exposes pre-computed metrics.
Issue ticket number and link
DPROD-1388