Handle scope name version #38413
Handle scope name version #38413
Conversation
There was a problem hiding this comment.
We also have a test case called duplicated scope name and version at the top of TestTranslatev2. It would be nice if it were grouped together with the new test at the bottom. While you're at it, could you also add comments with the index positions? It's hard to look at the labelsRef and immediately understand which labels they represent 😛
Also, rereading it, I feel like duplicated... sounds confusing 🤔 Where is the duplication? Something like timeseries with different scopes sounds more appropriate
| scopeName := prw.settings.BuildInfo.Description | ||
| scopeVersion := prw.settings.BuildInfo.Version | ||
|
|
||
| if sName := ls.Get("otel_scope_name"); sName != "" { | ||
| scopeName = sName | ||
| } | ||
|
|
||
| if sVersion := ls.Get("otel_scope_version"); sVersion != "" { | ||
| scopeVersion = sVersion | ||
| } |
There was a problem hiding this comment.
Could we make this a function? We'll repeat this code in all Add...Datapoints calls, so we might as well make this reusable instead of duplicated
Fixed cc705d5, looking directly in the git diff it's a little bit hard to understand that I just changed the order. Maybe will be easy to understand if you "open the branch" |
There was a problem hiding this comment.
Fixed cc705d5, looking directly in the git diff it's a little bit hard to understand that I just changed the order. Maybe will be easy to understand if you "open the branch"
Yeah, I did that :) Looks good
The failing test seems to be unrelated to your change.
dehaansa
added
the
ready to merge
Description
This PR ensures that when Prometheus samples lack the otel_scope_name or otel_scope_version labels, the receiver fills in default values.
It take care of part of #37277