-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor metrics so that everything is sent from Heartbeat in the backend #818
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #818 +/- ##
=======================================
Coverage 74.54% 74.54%
=======================================
Files 111 112 +1
Lines 13243 13271 +28
=======================================
+ Hits 9872 9893 +21
- Misses 2653 2658 +5
- Partials 718 720 +2 ☔ View full report in Codecov by Sentry. |
fa48cc8
to
f191230
Compare
@@ -106,9 +106,6 @@ type Backend interface { | |||
|
|||
// RedisPool returns the redisPool for this backend | |||
RedisPool() *redis.Pool | |||
|
|||
// CloudWatch return the CloudWatch service for this backend | |||
CloudWatch() *cwatch.Service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cloudwatch is now completely internal to the RP backend.. which seems right
f191230
to
3ea1c7d
Compare
3ea1c7d
to
1917dd2
Compare
cwatch.Datum("DBConnectionsInUse", float64(dbStats.InUse), cwtypes.StandardUnitCount, hostDim), | ||
cwatch.Datum("DBConnectionWaitDuration", float64(dbWaitDurationInPeriod/time.Millisecond), cwtypes.StandardUnitMilliseconds, hostDim), | ||
cwatch.Datum("DBConnectionWaitDuration", float64(dbWaitDurationInPeriod/time.Second), cwtypes.StandardUnitSeconds, hostDim), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're using seconds everywhere
@@ -131,6 +129,8 @@ func newBackend(cfg *courier.Config) courier.Backend { | |||
receivedExternalIDs: redisx.NewIntervalHash("seen-external-ids", time.Hour*24, 2), // 24 - 48 hours | |||
sentIDs: redisx.NewIntervalSet("sent-ids", time.Hour, 2), // 1 - 2 hours | |||
sentExternalIDs: redisx.NewIntervalHash("sent-external-ids", time.Hour, 2), // 1 - 2 hours | |||
|
|||
stats: NewStatsCollector(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to call Heartbeat when the backend stop or in backend cleanup so we are sure all the stats at that time are sent? it seems the stats in the last minute not yet send will be lost
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't worry about losing some metrics here and there.. but I do want to move the heartbeat stuff completely inside the backend in another pr and could think about sending final metrics when stopping courier.
No description provided.