Prometheus label conversion refactored to align with spec #7291
Conversation
Codecov ReportAttention: Patch coverage is
❌ Your patch check has failed because the patch coverage (56.86%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #7291 +/- ##
============================================
- Coverage 90.03% 89.94% -0.09%
- Complexity 6915 6925 +10
============================================
Files 787 787
Lines 20864 20914 +50
Branches 2023 2034 +11
============================================
+ Hits 18784 18812 +28
- Misses 1441 1459 +18
- Partials 639 643 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| * | ||
| * @return {@code true} if primitive, otherwise {@code false} | ||
| */ | ||
| public boolean isPrimitive() { |
There was a problem hiding this comment.
I'd recommend avoiding adding any new public API since that has to meet a pretty high bar for us
There was a problem hiding this comment.
Yup - let's just keep this as a utility method in Otel2PrometheusConverter.
| * | ||
| * @return {@code true} if primitive, otherwise {@code false} | ||
| */ | ||
| public boolean isPrimitive() { |
There was a problem hiding this comment.
This could be implemented as an EnumSet#contains, it would save some lines and could be slightly more efficient
|
|
||
| private static String maybeToJson(Object attributeValue) { | ||
| try { | ||
| return OBJECT_MAPPER.writeValueAsString(attributeValue); |
There was a problem hiding this comment.
Since we only need to serialize non-primitive types in AttributeType, maybe this could be done "by hand", with a simple StringBuilder and a loop? This PR seems to be the only reason to have Jackson in the classpath, so maybe it's worth considering at least
Resolves the issue #5987.