prometheus: Add instrumentation scope attributes to otel_scope_info #5932
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5932 +/- ##
=======================================
- Coverage 84.6% 84.6% -0.1%
=======================================
Files 272 272
Lines 22854 22860 +6
=======================================
+ Hits 19340 19344 +4
- Misses 3170 3172 +2
Partials 344 344 |
pellared
changed the title
pellared
changed the title
pellared
requested review from
MrAlias,
XSAM,
dashpole and
dmathieu
as code owners
| } | ||
| attrs = append(attrs, scope.Attributes.ToSlice()...) | ||
|
|
||
| keys, values := getAttrs(attribute.NewSet(attrs...), [2]string{}, [2]string{}, keyVals{}) |
There was a problem hiding this comment.
I thought about refactoring getAttrs into two functions like:
func getAttrs(attrs attribute.Set) ([]string, []string)
func getAttrsWithAppend(attrs attribute.Set, ks, vs [2]string, resourceKV keyVals) ([]string, []string)
OR
func getMetricAttrs(attrs attribute.Set, ks, vs [2]string, resourceKV keyVals) ([]string, []string)but I resisted as I thought it may be too much in a single PR
We currently have getAttrs(XYZ), [2]string{}, [2]string{}, keyVals{}) call in three places: createInfoMetric, createScopeInfoMetric, createResourceAttributes.
The getAttrs(dp.Attributes, ks, vs, resourceKV) calls are used in: addHistogramMetric, addSumMetric, addGaugeMetric.
Let me know if I should address it in this PR, separate PR, or should I leave as it is.
My preference is to make a separate PR for it as I am not sure about naming "the longer" function. Also it should make the PRs easier for review.
There was a problem hiding this comment.
@dashpole, I am asking for your guidance and preferences 🙏
There was a problem hiding this comment.
Here is my proposal on how to refactor it: #5937
Fixes #5846
Pre-work: #5806