chore: oauth v2 stats refactor #5262
Conversation
| ) (int, *AuthResponse, error) { | ||
| actionType := strings.Join(strings.Fields(strings.ToLower(logTypeName)), "_") |
There was a problem hiding this comment.
For reference
> strings.Join(strings.Fields(strings.ToLower("Refresh Token")), "_")
refresh_token
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5262 +/- ##
==========================================
+ Coverage 74.74% 74.79% +0.04%
==========================================
Files 433 433
Lines 60926 60942 +16
==========================================
+ Hits 45542 45580 +38
+ Misses 12862 12846 -16
+ Partials 2522 2516 -6 ☔ View full report in Codecov by Sentry. |
|
This PR is considered to be stale. It has been open 20 days with no further activity thus it is going to be closed in 7 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR. |
| func (m *OAuthStatsHandler) mergeTags(tags stats.Tags) stats.Tags { | ||
| allTags := m.defaultTags | ||
| for key, value := range tags { | ||
| allTags[key] = value | ||
| } | ||
| s.stats.NewTaggedStat(s.statName, stats.CountType, statsTags).Increment() | ||
| return allTags | ||
| } |
There was a problem hiding this comment.
This should be the other way around. If we don't want to put values into m.defaultTags.
func (m *OAuthStatsHandler) mergeTags(tags stats.Tags) stats.Tags {
for key, value := range m.defaultTags {
tags[key] = value
}
return tags
}
Add a map lookup check if you don't want to override with default.
| func (m *OAuthStatsHandler) getStatName(suffix string) string { | ||
| return strings.Join([]string{"oauth_action", suffix}, "_") | ||
| } |
There was a problem hiding this comment.
minor
Can inline this function's call.
Description
The way in which stats are written is not easily readable. Through this PR, we are trying to make the stat emission in oauthv2 module a bit more developer friendly and readable
Linear Ticket
Resolves INT-2744
Security