Metrics! (closes #175) #251
Conversation
As long as we can correlate a "finished" event to a "started doing something event," timer contexts should be an implementation detail for listeners.
Dropped `handleNotificationSendAttemptStarted`, too, which seemed like it was a bit over the line in terms of confusion/benefit ratio.
This should significantly reduce the amount of synchronization we have to do.
| // We only want to begin a connection attempt if one is not already in progress or complete; if we already | ||
| // have a connection future, just return the existing promise. | ||
| if (this.connectionReadyPromise == null) { | ||
| if (metricsListener != null) { |
There was a problem hiding this comment.
I would be inclined to create a NullMetricsListener class which just contains no-ops, and use that by default if setMetricsListener() hasn't been called, in order to avoid the need to null-check this on each call (as in NullMetrics in https://github.com/relayrides/pushy/pull/222/files#diff-bd45fb4e15d6ed613f0e6f1e1d5a193fR6 ).
There was a problem hiding this comment.
Fair enough. Done in 7079143.
|
looks great! |
|
I think I'm going to punt on a reference implementation for now, at least in part because I don't want to hold up the v0.6 release while I figure out how to do a multi-module release with Maven. I'm also thinking it could make sense to do vendor-specific implementations as entirely separate projects. I'm open to suggestions here, though! |
|
🎉 looks awesome. looking forward to using this ;) |
This adds an interface callers can use to listen for performance metrics from
ApnsClients. The goal here is to expose important metrics without introducing dependencies on specific metrics libraries.TODO:
Decide where to put reference implementations(deferred)