Skip to content

Fix/metric emission tracking #2825#2870

Closed
snehavats1404 wants to merge 5 commits into
apache:masterfrom
snehavats1404:fix/metric-emission-tracking
Closed

Fix/metric emission tracking #2825#2870
snehavats1404 wants to merge 5 commits into
apache:masterfrom
snehavats1404:fix/metric-emission-tracking

Conversation

@snehavats1404

@snehavats1404 snehavats1404 commented Jan 13, 2025

Copy link
Copy Markdown

What problem does this PR solve?

Issue Number: #2825

Problem Summary:
The issue pertains to incorrectly tracking and emitting metric names when dumping Prometheus metrics. Some metrics, especially those related to latency, were not being properly emitted due to incorrect handling of the emitted_metrics set.

What is changed and the side effects?

Changed:

  • Fixed the logic for tracking emitted metrics by using a set to ensure that duplicate metrics are not emitted.
  • Ensured that all necessary latency percentiles and summary metrics are properly output, with correct handling of the _latency_* and _count suffixes.
  • Added a mechanism to track previously emitted metric names to prevent duplication in Prometheus output.

Side effects:

  • Performance effects: The introduction of a set to track emitted metrics could slightly impact performance, but this change ensures more efficient handling of duplicate metrics.
  • Breaking backward compatibility: No breaking changes; existing behavior should remain intact except for the fix to prevent duplicate metric emission.

@snehavats1404 snehavats1404 changed the title Fix/metric emission tracking #2885 Fix/metric emission tracking #2825 Jan 13, 2025
Comment on lines +106 to +110
if(emitted_metrics.find(metrics_name.as_string()) == emitted_metrics.end()){
*_os<< "# HELP " << metrics_name <<'\n'
<< "# TYPE " << metrics_name <<" gauge\n";
emitted_metrics.insert(metrics_name.as_string());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some spaces are missing.

@chenBright

chenBright commented Jan 13, 2025

Copy link
Copy Markdown
Contributor
  1. If the bvar of mbvar is not continuously input into the dump, is there a problem?
  2. In addition, it is necessary to add unit test cases.

@snehavats1404

Copy link
Copy Markdown
Author

I don't feel there must be any issue in not adding any unit testcase, if there is any please let me know.

@chenBright

Copy link
Copy Markdown
Contributor

I don't feel there must be any issue in not adding any unit testcase, if there is any please let me know.

We hope that PR will be equipped with unit tests as much as possible to ensure correctness.

@chenBright

chenBright commented Mar 12, 2025

Copy link
Copy Markdown
Contributor

Close this as completed in #2899 .

@chenBright chenBright closed this Mar 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

brpc的brpc_metrics的输出格式与prometheus的标准不符,尤其MultiDimension<bvar::LatencyRecorder>方式

2 participants