Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[KYUUBI #6866][FOLLOWUP] Fix get existing gauge #6891

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

turboFei
Copy link
Member

@turboFei turboFei commented Jan 15, 2025

Why are the changes needed?

For the com.codahale.metrics.MetricRegistry::gauge.
It getOrAdd the gauge with name.

    public <T extends Gauge> T gauge(String name) {
        return (Gauge)this.getOrAdd(name, MetricRegistry.MetricBuilder.GAUGES);
    }

So we have to get all the gauges to check whether the gauge exists.

How was this patch tested?

UT.

Was this patch authored or co-authored using generative AI tooling?

No.

@turboFei turboFei requested a review from pan3793 January 15, 2025 01:47
@turboFei turboFei self-assigned this Jan 15, 2025
@turboFei turboFei modified the milestones: v1.9.4, v1.10.2 Jan 15, 2025
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (fff1841) to head (039e7b5).

Files with missing lines Patch % Lines
...cala/org/apache/kyuubi/metrics/MetricsSystem.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master   #6891   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         688     688           
  Lines       42590   42590           
  Branches     5805    5805           
======================================
  Misses      42590   42590           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants