metrics: Add Metrics tags to multiple Scopes #2687
Conversation
|
thanks @albertorm95 do you think you could give it a try and try to fix that error? we recently upgraded to go1.19 I wonder if is related to new client instantiations that meed updating |
|
@albertorm95 please add tests for this |
Hello @nitrocode , do you want me to create a test that verifies that the tags are in the metric? |
|
Any tests would be helpful to prevent breaking the feature. I do not recall if we added any tests for the metrics feature in general.. |
albertorm95
force-pushed
the
feat-add-metrics-tags
branch
from
0fc36d8
to
ca15bad
Compare
| @@ -235,3 +243,10 @@ func fmtLogSrc(repo models.Repo, pullNum int) []interface{} { | |||
| "pull-num", strconv.Itoa(pullNum), | |||
| } | |||
| } | |||
|
|
|||
| func SetGitScopeTags(scope tally.Scope, repoFullName string, pullNum int) tally.Scope { | |||
There was a problem hiding this comment.
Is there a global place to put some functions like helpers?
| // instead of a pointer since we want scopes to mirror our function stack | ||
| func (p ProjectContext) SetScope(scope string) { | ||
| // SetScopeTags adds ProjectContext tags to a new returned scope. | ||
| func (p ProjectContext) SetScopeTags(scope tally.Scope) tally.Scope { |
There was a problem hiding this comment.
After a lot of debugging I got to the conclusion that the previous code was doing nothing, so instead of creating a function to SetScope that was basically scope.SubScope, I renamed to SetScopeTags that will add the listed tags to the scope
There was a problem hiding this comment.
This looks good to me but let's get a second opinion too. cc: @jamengual @chenrui333 @lilincmu
|
Thank you @albertorm95 ! |
* metrics: Add Metrics tags to multiple Scopes * metrics: Fix base_repo and pr_number * metrics: Refactor SetScope to SetScopeTags * metrics: Add SetGitScopeTags function * metrics: docs reword project_context SetScopeTags description * metrics: test Add TestNewScope_PrometheusTaggingCapabilities
Follow up from:
Before:
Metrics:
After:
Metrics: