chore: add reason tag in failed requests and failed events stats #2430
Conversation
Codecov ReportBase: 45.30% // Head: 43.96% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2430 +/- ##
==========================================
- Coverage 45.30% 43.96% -1.35%
==========================================
Files 177 187 +10
Lines 36542 39108 +2566
==========================================
+ Hits 16555 17193 +638
- Misses 18911 20828 +1917
- Partials 1076 1087 +11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
gateway/gateway.go
Outdated
| @@ -1079,7 +1101,7 @@ func (gateway *HandleT) getPayloadAndWriteKey(_ http.ResponseWriter, r *http.Req | |||
| if !ok || writeKey == "" { | |||
| err = errors.New(response.NoWriteKeyInBasicAuth) | |||
| misc.IncrementMapByKey(sourceFailStats, "noWriteKey", 1) | |||
| gateway.updateSourceStats(sourceFailStats, "gateway.write_key_failed_requests", map[string]string{"noWriteKey": "noWriteKey", "reqType": reqType}) | |||
| gateway.updateFailedSourceStats(sourceFailStats, "gateway.write_key_failed_requests", map[string]string{"noWriteKey": "noWriteKey", "reqType": reqType, "reason": "noWriteKeyInBasicAuth"}) | |||
There was a problem hiding this comment.
Some formatting to make the long line more readable:
| gateway.updateFailedSourceStats(sourceFailStats, "gateway.write_key_failed_requests", map[string]string{"noWriteKey": "noWriteKey", "reqType": reqType, "reason": "noWriteKeyInBasicAuth"}) | |
| gateway.updateFailedSourceStats(sourceFailStats, "gateway.write_key_failed_requests", map[string]string{ | |
| "noWriteKey": "noWriteKey", | |
| "reqType": reqType, | |
| "reason": "noWriteKeyInBasicAuth" | |
| }) |
| gateway.updateSourceStats(sourceSuccessStats, "gateway.write_key_successful_requests", sourceTagMap) | ||
| gateway.updateSourceStats(sourceFailStats, "gateway.write_key_failed_requests", sourceTagMap) | ||
| gateway.updateFailedSourceStats(sourceFailStats, "gateway.write_key_failed_requests", sourceTagMap) |
There was a problem hiding this comment.
I find it peculiar having 3 different metrics that capture similar things, with an introduction of a tag we can simplify things.
What if gateway.write_key_requests had two extra tags:
failed: true/falsereason- same to what you introduce here.
Using the failed tag above you can get any metric you like.
With this, we need can simplify our code and make our stats more compact.
We still need to keep:
gateway.updateSourceStats(sourceSuccessStats, "gateway.write_key_successful_requests", sourceTagMap)
gateway.updateFailedSourceStats(sourceFailStats, "gateway.write_key_failed_requests", sourceTagMap)For a few release for backwards compatibility, but after that, they can be removed.
There was a problem hiding this comment.
@lvrach The similar metrics exist for gateway events, gateway.write_key_events, gateway.write_key_successful_events, gateway.write_key_failed_events
Let's take this a separate change ?
Also, I think we might need to revisit all the metrics once. I think we need to add rudderstack resourceIDs (sourceID, destinationID, transformationID, trackingPlanID) in the tags wherever possible. Going forward, this would enable us to send more notifications to the customers. With your changes to filter out these tags in free this should not be an issue ?
satishrudderstack
requested review from
cisse21,
atzoum and
rudder-server-bot
| @@ -244,7 +244,7 @@ func Run(ctx context.Context) int { | |||
| } | |||
| // TODO: remove as soon as we update the configuration with statsExcludedTags where necessary | |||
There was a problem hiding this comment.
Can someone explain this TODO to me?
There was a problem hiding this comment.
Was discussing the same with @chandumlg ... So the basic idea is that for free customers we don't want to send so many tags to influx as it gets bloated up but essentially we should remove the redundant tags for every metric
Currently, we have stats for failed requests and failed events. With this change we will also be capturing the reason for the failure. Example reasons are invalidJson, invalidWriteKey