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

Allow customizing the MeterIdPrefix of CircuitBreaker metrics(#5318) #5383

Closed

Conversation

Kyurenpoto
Copy link
Contributor

Motivation:

To resolve #5318:

Currently, the metric name and tags of circuit breaker metrics are determined by MetricCollectingCircuitBreakerListener.metricsOf(String circuitBreakerName). The resulting meter ID always contains the name of the circuit breaker.

When a user uses a CircuitBreakerMapping such as perHost(), each circuit breaker will have its own metrics under its own meter ID prefix, which may or may not be something a user desires.

We could allow a user to specify a function that allows a user to choose the MeterIdPrefix of the circuit breaker so that a user can use the same MeterIdPrefix while using per-host circuit breakers.

Modifications:

  • Extract MetricTagBuilder
  • Add constructor to specify custom tags

Result:

We could allow a user to specify the tags that allows a user to choose the MeterIdPrefix of the circuit breaker.

Copy link

codecov bot commented Jan 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.68%. Comparing base (3f54be0) to head (d46f1e8).
Report is 198 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5383      +/-   ##
============================================
- Coverage     73.94%   73.68%   -0.27%     
- Complexity    20104    20107       +3     
============================================
  Files          1730     1741      +11     
  Lines         74161    74366     +205     
  Branches       9465     9481      +16     
============================================
- Hits          54841    54799      -42     
- Misses        14844    15033     +189     
- Partials       4476     4534      +58     

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

@Kyurenpoto Kyurenpoto force-pushed the feature/circuit-breaker-meter-id-prefix branch from 81c34c0 to 02f24a2 Compare January 16, 2024 07:43
@Kyurenpoto Kyurenpoto force-pushed the feature/circuit-breaker-meter-id-prefix branch from 0df1a97 to d46f1e8 Compare January 16, 2024 08:55
@ikhoon ikhoon added this to the 1.28.0 milestone Jan 16, 2024
@Kyurenpoto Kyurenpoto force-pushed the feature/circuit-breaker-meter-id-prefix branch 2 times, most recently from 46756e5 to d46f1e8 Compare January 25, 2024 01:33
@jrhee17 jrhee17 modified the milestones: 1.28.0, 1.29.0 Apr 8, 2024
@github-actions github-actions bot added Stale and removed Stale labels May 9, 2024
@minwoox
Copy link
Member

minwoox commented May 16, 2024

I left my opinion here: #5318 (comment)

@minwoox minwoox modified the milestones: 1.29.0, 1.30.0 May 21, 2024
@trustin trustin removed this from the 1.30.0 milestone Jun 6, 2024
@trustin
Copy link
Member

trustin commented Jun 6, 2024

I think we should go with Micrometer Observation as described at #5736. Sorry but let me close this. 🙇

@trustin trustin closed this Jun 6, 2024
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.

Allow customizing the MeterIdPrefix of CircuitBreaker metrics.
5 participants