paultanay commented on PR #4303: URL: https://github.com/apache/hertzbeat/pull/4303#issuecomment-5320376184
> ## Review: feat(collector): support milvus collector (#3877) > **Verdict: ✅ APPROVED** — useful new monitor template; please address two doc/metric notes before or after merge. > > ### What this PR does > Adds a Milvus vector-DB monitor template (`app-milvus.yml`) scraping the Prometheus `/metrics` endpoint on port 9091, plus the matching help doc. Covers proxy request/slow-query/vector counts, search & mutation latency, QueryNode loaded collections/entities, and RootCoord collection/partition/DML/DDL counts across 12 priority groups. > > ### Assessment > * **Structure**: Valid template — ASF license header, trilingual `name`/`help`, sensible `params` (host/port/timeout), and consistent `protocol: http` + `parseType: prometheus` per group. Port default 9091 and path `/metrics` match Milvus defaults. > * **Risk**: Low — additive, no code path touched. > > ### Notes (non-blocking) > 1. **Doc/metric mismatch**: `home/docs/help/milvus.md` lists metrics that the template does **not** define (e.g. `milvus_proxy_sq_latency_sum`, `milvus_proxy_receive_bytes_count`, `milvus_proxy_send_bytes_count`, `milvus_querynode_search_req_latency_sum`, `milvus_querynode_search_group_nq_sum`, `milvus_querynode_evicted_memory_size`). The "Collected Metrics" tables read like Milvus's full metric surface rather than what this template actually scrapes. Please align the doc tables with the 12 metric names present in `app-milvus.yml` so users aren't misled. > 2. **Histogram metric names**: `milvus_proxy_sq_latency` and `milvus_proxy_mutation_latency` are modeled with an `le` label (classic histogram-bucket shape). Prometheus histograms expose `..._bucket` / `..._sum` / `..._count` series; a bare `milvus_proxy_sq_latency` series usually does not exist, so these two groups may silently collect nothing. Please verify Milvus emits a bare series with the `le` label, or retarget the metric name to `milvus_proxy_sq_latency_bucket` (and add `_sum`/`_count` as needed). > > Nice addition overall. addressed remaining suggestions -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
