seawinde opened a new pull request, #63485:
URL: https://github.com/apache/doris/pull/63485

   ### What problem does this PR solve?
   
   Issue Number: N/A
   
   Related PR: #13832
   
   Problem Summary:
   Histogram metrics that encode labels in the Dropwizard metric name can be 
rendered incorrectly when label values contain dots or other special 
characters. For example, a user value like `[email protected]` can be split as 
part of the metric name instead of being kept as the `user` label.
   
   Root cause: In `PrometheusMetricVisitor.visitHistogram()` and 
`JsonMetricVisitor.visitHistogram()`, the legacy histogram export path splits 
the Dropwizard histogram name by dots and infers labels from `k=v` name 
segments. `MetricRepo.USER_HISTO_QUERY_LATENCY` previously embedded the user 
label into the histogram name, so dots inside user names were treated as 
metric-name separators.
   
   Change Summary:
   
   | File | Change Description |
   |------|--------------------|
   | `fe/fe-core/src/main/java/org/apache/doris/metric/HistogramMetric.java` | 
Add a small wrapper that keeps the Dropwizard `Histogram` together with its 
metric name and structured labels. |
   | `fe/fe-core/src/main/java/org/apache/doris/metric/MetricRepo.java` | 
Export labeled histogram metrics through the new structured label path while 
keeping existing static histograms exported. |
   | `fe/fe-core/src/main/java/org/apache/doris/metric/CloudMetrics.java` | 
Keep cloud query latency and meta-service RPC latency labels structured instead 
of encoding them in histogram names. |
   | `fe/fe-core/src/main/java/org/apache/doris/metric/MetricVisitor.java` | 
Add a labeled histogram visit overload. |
   | 
`fe/fe-core/src/main/java/org/apache/doris/metric/PrometheusMetricVisitor.java` 
| Render structured histogram labels directly in Prometheus output. |
   | `fe/fe-core/src/main/java/org/apache/doris/metric/JsonMetricVisitor.java` 
| Render structured histogram labels directly in JSON output. |
   | `fe/fe-core/src/test/java/org/apache/doris/metric/MetricsTest.java` | 
Cover user and cloud histogram labels containing dots and at signs. |
   
   Design Rationale:
   The fix follows the existing counter and gauge approach: keep metric names 
stable and carry dimensions as `MetricLabel` values. The legacy three-argument 
histogram visitor remains for existing static Dropwizard histograms without 
labels, while dynamic histograms that need labels use the structured 
four-argument path.
   
   ```mermaid
   graph TD
       A[Query finished] --> B[AuditLogHelper.updateMetricsImpl]
       B --> C[USER_HISTO_QUERY_LATENCY.getOrAdd(user)]
       C --> D[HistogramMetric name + Histogram + labels]
       D --> E[MetricRepo.visitHistograms]
       E --> F[PrometheusMetricVisitor / JsonMetricVisitor]
       F --> G[Stable metric name with structured labels]
   ```
   
   ### Release note
   
   Fixed an issue where FE histogram metrics with label values containing dots, 
such as user names or cloud cluster names, could be exported with malformed 
metric names or labels.
   
   ### Check List (For Author)
   
   - Test
       - [ ] Regression test
       - [x] Unit Test
       - [ ] Manual test (add detailed scripts or steps below)
       - [ ] No need to test or manual test. Explain why:
           - [ ] This is a refactor/code format and no logic has been changed.
           - [ ] Previous test can cover this change.
           - [ ] No code files have been changed.
           - [ ] Other reason
   
   Unit tests:
   - Maven package for `fe-core` passed with tests skipped to verify compile 
and checkstyle.
   - `org.apache.doris.metric.MetricsTest` passed manually, `OK (12 tests)`.
   - `./run-fe-ut.sh --run org.apache.doris.metric.MetricsTest` was attempted 
but blocked by an unrelated testCompile classpath issue where 
`CloudProcVersionDisplayTest` imports `mockit`.
   
   - Behavior changed:
       - [x] No. This restores intended metric label rendering.
       - [ ] Yes.
   
   - Does this need documentation?
       - [x] No.
       - [ ] Yes.
   
   ### Check List (For Reviewer who merge this PR)
   
   - [ ] Confirm the release note
   - [ ] Confirm test cases
   - [ ] Confirm document
   - [ ] Add branch pick label
   


-- 
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]

Reply via email to