Set a common group tag #130
Conversation
|
@jonatan-ivanov would like your thoughts if you have any knowledge here. If you think this looks OK, I would probably make this configurable as well as add a property to disable it. |
Codecov Report
@@ Coverage Diff @@
## 2.1.x #130 +/- ##
============================================
+ Coverage 77.39% 77.56% +0.17%
- Complexity 78 81 +3
============================================
Files 13 14 +1
Lines 345 361 +16
Branches 10 12 +2
============================================
+ Hits 267 280 +13
- Misses 74 77 +3
Partials 4 4
Continue to review full report at Codecov.
|
|
|
||
| @Bean | ||
| MeterRegistryCustomizer<MeterRegistry> metricsCommonTags() { | ||
| return registry -> registry.config().commonTags(Resilience4JCircuitBreaker.CIRCUIT_BREAKER_GROUP_TAG, "none"); |
There was a problem hiding this comment.
This way the tag will be added for every single meter you have (e.g.: Spring MVC metrics, Kafka metrics, etc). Is it what you want or do you want to only add this tag to Resilience4J-related metrics?
We usually recommend adding fallback tag values at the instrumentation level, e.g.:
Timer timer = Timer.builder("my.timer")
.tags("group", circuitBreaker.getGroupName() != null ? circuitBreaker.getGroupName() : "none")
Here's a real-life example when we attach the name of the exception as a tag but we set value "none" if there were no exceptions: TimedAspect.java#L216-L227. I guess your use-case is similar.
If you have no control over this, I recommend using a MeterFilter where you can selectively attach the missing tag based on the name and the current tags of the Meter.
There was a problem hiding this comment.
This way the tag will be added for every single meter you have (e.g.: Spring MVC metrics, Kafka metrics, etc). Is it what you want or do you want to only add this tag to Resilience4J-related metrics?
No just resilience4j metrics, is there a better way to do that?
If you have no control over this, I recommend using a MeterFilter where you can selectively attach the missing tag based on the name and the current tags of the Meter.
I think the problem is here https://github.com/resilience4j/resilience4j/blob/master/resilience4j-spring/src/main/java/io/github/resilience4j/bulkhead/configure/threadpool/ThreadPoolBulkheadConfiguration.java#L72-L74
Which we don't have control over. They are not using the tags like we are.
There was a problem hiding this comment.
I would suggest creating a MeterFilter (boot will auto-config it for you), like this: https://micrometer.io/docs/concepts#_transforming_metrics
So you can check if this is a resilience4j-related meter and the group tag is missing, add it with a default value.
Use MeterFilter instead of MeterRegistryCustomizer
ryanjbaxter
force-pushed
the
group-name-default-name
branch
from
0145560 to
14fa251
Compare
Fixes #121
This addresses an issue in micrometer where CB with different tags end up not showing up in the metrics.
See micrometer-metrics/micrometer#877