[PR] Fix BanyanDB property merge-latency unit (nanoseconds, not seconds) [skywalking]

["wu-sheng (via GitHub)" <gi...@apache.org>]

wu-sheng opened a new pull request, #13906:
URL: https://github.com/apache/skywalking/pull/13906

   ### Fix `meter_banyandb_endpoint_property_index_merge_latency` over-reporting by 1,000,000×
   
   - [ ] Add a unit test to verify that the fix works. — The MAL test asserts entities (not metric *values*) and the boot-check asserts the rule compiles; the value-correctness here is established by the BanyanDB **source unit** (see below), which a rule unit test can't assert. Boot-check (`DSLClassGeneratorTest`) green.
   - [x] Explain briefly why the bug exists and how to fix it.
   
   **Why:** `property_index_merge_latency` converted its source with `* 1000` (seconds → ms), like the `*_tst_total_merge_latency` rules. But `banyandb_property_inverted_index_total_merge_latency` is **not** in seconds — it's bluge's inverted-index `ZapTime`, set directly from `float64(TotFileMergeZapTime)` (`pkg/index/inverted/metrics.go:151,156`), which bluge computes as `uint64(time.Since(...))` (`index/merge.go`) — a raw `time.Duration` in **nanoseconds**, never `.Seconds()`. So the rule over-reported merge latency by **1e6**.
   
   **Fix:** convert ns → ms with `/ 1000000` (one line + a source-cited comment).
   
   **Scope check:** I audited *every* BanyanDB latency rule across the three `banyandb-*.yaml` files against the source. All others are correct — every other family records `.Seconds()`; the histogram `*_latency_p99` rules get an implicit seconds → ms from MAL's `histogram()` default `SECONDS` bucket unit (`le * toMillis(1)`), so they correctly omit `*1000`; the `safeDiv * 1000` rules (query / merge-file latency) match their seconds source. **`property_index_merge_latency` was the only mismatch.**
   
   - [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue number.
   - [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/docs/en/changes/changes.md). — N/A: `property_index_merge_latency` is part of the **unreleased** SWIP-15 so11y rules (11.0.0, merged in #13905, not yet released), so the bug never shipped; covered by that entry.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)


-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Fix BanyanDB property merge-latency unit (nanoseconds, not seconds) [skywalking]

["wu-sheng (via GitHub)" <gi...@apache.org>]

wu-sheng merged PR #13906:
URL: https://github.com/apache/skywalking/pull/13906


-- 
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: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org