diff --git a/dropwizard-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListener.java b/dropwizard-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListener.java index 9a5cdf9f1..167b33716 100644 --- a/dropwizard-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListener.java +++ b/dropwizard-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListener.java @@ -25,10 +25,10 @@ import com.codahale.metrics.*; import com.eatthepath.pushy.apns.ApnsClient; import com.eatthepath.pushy.apns.ApnsClientMetricsListener; +import com.eatthepath.pushy.apns.PushNotificationResponse; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; /** @@ -78,7 +78,6 @@ public class DropwizardApnsClientMetricsListener implements ApnsClientMetricsLis private final MetricRegistry metrics; private final Timer notificationTimer; - private final ConcurrentMap notificationTimerContexts; private final Meter writeFailures; private final Meter sentNotifications; @@ -145,7 +144,6 @@ public DropwizardApnsClientMetricsListener() { this.metrics = new MetricRegistry(); this.notificationTimer = this.metrics.timer(NOTIFICATION_TIMER_NAME); - this.notificationTimerContexts = new ConcurrentHashMap<>(); this.writeFailures = this.metrics.meter(WRITE_FAILURES_METER_NAME); this.sentNotifications = this.metrics.meter(SENT_NOTIFICATIONS_METER_NAME); @@ -161,96 +159,67 @@ public DropwizardApnsClientMetricsListener() { /** * Records a failed attempt to send a notification and updates metrics accordingly. * - * @param apnsClient the client that failed to write the notification; note that this is ignored by - * {@code DropwizardApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that could not be written + * @param topic the APNs topic to which the notification was sent */ @Override - public void handleWriteFailure(final ApnsClient apnsClient, final long notificationId) { - this.stopTimerForNotification(notificationId); + public void handleWriteFailure(final String topic) { this.writeFailures.mark(); } /** * Records a successful attempt to send a notification and updates metrics accordingly. * - * @param apnsClient the client that sent the notification; note that this is ignored by - * {@code DropwizardApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that was sent + * @param topic the APNs topic to which the notification was sent */ @Override - public void handleNotificationSent(final ApnsClient apnsClient, final long notificationId) { + public void handleNotificationSent(final String topic) { this.sentNotifications.mark(); - this.notificationTimerContexts.put(notificationId, this.notificationTimer.time()); } /** - * Records that the APNs server accepted a previously-sent notification and updates metrics accordingly. + * Records that the APNs server acknowledged a previously-sent notification and updates metrics accordingly. * - * @param apnsClient the client that sent the accepted notification; note that this is ignored by - * {@code DropwizardApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that was accepted + * @param response the response from the APNs server + * @param durationNanos the duration, in nanoseconds, between the time the notification was initially sent and when + * it was acknowledged by the APNs server */ @Override - public void handleNotificationAccepted(final ApnsClient apnsClient, final long notificationId) { - this.stopTimerForNotification(notificationId); - this.acceptedNotifications.mark(); - } + public void handleNotificationAcknowledged(final PushNotificationResponse response, final long durationNanos) { + if (response.isAccepted()) { + this.acceptedNotifications.mark(); + } else { + this.rejectedNotifications.mark(); + } - /** - * Records that the APNs server rejected a previously-sent notification and updates metrics accordingly. - * - * @param apnsClient the client that sent the rejected notification; note that this is ignored by - * {@code DropwizardApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that was rejected - */ - @Override - public void handleNotificationRejected(final ApnsClient apnsClient, final long notificationId) { - this.stopTimerForNotification(notificationId); - this.rejectedNotifications.mark(); + this.notificationTimer.update(durationNanos, TimeUnit.NANOSECONDS); } /** * Records that the APNs server added a new connection to its internal connection pool and updates metrics * accordingly. - * - * @param apnsClient the client that added the new connection */ @Override - public void handleConnectionAdded(final ApnsClient apnsClient) { + public void handleConnectionAdded() { this.openConnections.incrementAndGet(); } /** * Records that the APNs server removed a connection from its internal connection pool and updates metrics * accordingly. - * - * @param apnsClient the client that removed the connection */ @Override - public void handleConnectionRemoved(final ApnsClient apnsClient) { + public void handleConnectionRemoved() { this.openConnections.decrementAndGet(); } /** * Records that a previously-started attempt to connect to the APNs server failed and updates metrics accordingly. - * - * @param apnsClient the client that failed to connect; note that this is ignored by - * {@code DropwizardApnsClientMetricsListener} instances, which should always be used for exactly one client */ @Override - public void handleConnectionCreationFailed(final ApnsClient apnsClient) { + public void handleConnectionCreationFailed() { this.connectionFailures.mark(); } - private void stopTimerForNotification(final long notificationId) { - final Timer.Context timerContext = this.notificationTimerContexts.remove(notificationId); - - if (timerContext != null) { - timerContext.stop(); - } - } - /** * Returns the metrics produced by this listener. * diff --git a/dropwizard-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListenerTest.java b/dropwizard-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListenerTest.java index 67abb7e11..ffa72ebba 100644 --- a/dropwizard-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListenerTest.java +++ b/dropwizard-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListenerTest.java @@ -26,10 +26,15 @@ import com.codahale.metrics.Meter; import com.codahale.metrics.Metric; import com.codahale.metrics.Timer; +import com.eatthepath.pushy.apns.ApnsPushNotification; +import com.eatthepath.pushy.apns.PushNotificationResponse; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import java.time.Instant; import java.util.Map; +import java.util.Optional; +import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -48,7 +53,7 @@ public void testHandleWriteFailure() { final Meter writeFailures = (Meter) this.listener.getMetrics().get(DropwizardApnsClientMetricsListener.WRITE_FAILURES_METER_NAME); assertEquals(0, writeFailures.getCount()); - this.listener.handleWriteFailure(null, 1); + this.listener.handleWriteFailure("com.example.topic"); assertEquals(1, writeFailures.getCount()); } @@ -57,7 +62,7 @@ public void testHandleNotificationSent() { final Meter sentNotifications = (Meter) this.listener.getMetrics().get(DropwizardApnsClientMetricsListener.SENT_NOTIFICATIONS_METER_NAME); assertEquals(0, sentNotifications.getCount()); - this.listener.handleNotificationSent(null, 1); + this.listener.handleNotificationSent("com.example.topic"); assertEquals(1, sentNotifications.getCount()); } @@ -66,7 +71,7 @@ public void testHandleNotificationAccepted() { final Meter acceptedNotifications = (Meter) this.listener.getMetrics().get(DropwizardApnsClientMetricsListener.ACCEPTED_NOTIFICATIONS_METER_NAME); assertEquals(0, acceptedNotifications.getCount()); - this.listener.handleNotificationAccepted(null, 1); + this.listener.handleNotificationAcknowledged(buildPushNotificationResponse(true), 1); assertEquals(1, acceptedNotifications.getCount()); } @@ -75,7 +80,7 @@ public void testHandleNotificationRejected() { final Meter rejectedNotifications = (Meter) this.listener.getMetrics().get(DropwizardApnsClientMetricsListener.REJECTED_NOTIFICATIONS_METER_NAME); assertEquals(0, rejectedNotifications.getCount()); - this.listener.handleNotificationRejected(null, 1); + this.listener.handleNotificationAcknowledged(buildPushNotificationResponse(false), 1); assertEquals(1, rejectedNotifications.getCount()); } @@ -84,11 +89,11 @@ public void testHandleConnectionAddedAndRemoved() { @SuppressWarnings("unchecked") final Gauge openConnectionGauge = (Gauge) this.listener.getMetrics().get(DropwizardApnsClientMetricsListener.OPEN_CONNECTIONS_GAUGE_NAME); - this.listener.handleConnectionAdded(null); + this.listener.handleConnectionAdded(); assertEquals(1, (long) openConnectionGauge.getValue()); - this.listener.handleConnectionRemoved(null); + this.listener.handleConnectionRemoved(); assertEquals(0, (long) openConnectionGauge.getValue()); } @@ -98,7 +103,7 @@ public void testHandleConnectionCreationFailed() { final Meter connectionFailures = (Meter) this.listener.getMetrics().get(DropwizardApnsClientMetricsListener.CONNECTION_FAILURES_METER_NAME); assertEquals(0, connectionFailures.getCount()); - this.listener.handleConnectionCreationFailed(null); + this.listener.handleConnectionCreationFailed(); assertEquals(1, connectionFailures.getCount()); } @@ -116,4 +121,38 @@ public void testGetMetrics() { assertTrue(metrics.get(DropwizardApnsClientMetricsListener.OPEN_CONNECTIONS_GAUGE_NAME) instanceof Gauge); assertTrue(metrics.get(DropwizardApnsClientMetricsListener.CONNECTION_FAILURES_METER_NAME) instanceof Meter); } + + private static PushNotificationResponse buildPushNotificationResponse(final boolean accepted) { + return new PushNotificationResponse() { + @Override + public ApnsPushNotification getPushNotification() { + return null; + } + + @Override + public boolean isAccepted() { + return accepted; + } + + @Override + public UUID getApnsId() { + return null; + } + + @Override + public int getStatusCode() { + return 0; + } + + @Override + public Optional getRejectionReason() { + return Optional.empty(); + } + + @Override + public Optional getTokenInvalidationTimestamp() { + return Optional.empty(); + } + }; + } } diff --git a/micrometer-metrics-listener/README.md b/micrometer-metrics-listener/README.md index 3a71bc9b8..8d62d0efd 100644 --- a/micrometer-metrics-listener/README.md +++ b/micrometer-metrics-listener/README.md @@ -18,8 +18,7 @@ Creating new Micrometer listeners is straightforward. To get started, construct ```java final MicrometerApnsClientMetricsListener listener = - new MicrometerApnsClientMetricsListener(existingMeterRegistry, - "notifications", "apns", "pushy"); + new MicrometerApnsClientMetricsListener(existingMeterRegistry); final ApnsClient apnsClient = new ApnsClientBuilder() .setApnsServer(ApnsClientBuilder.DEVELOPMENT_APNS_HOST) @@ -29,7 +28,7 @@ final ApnsClient apnsClient = new ApnsClientBuilder() .build(); ``` -Note that a `MicrometerApnsClientMetricsListener` is intended for use with only one `ApnsClient` at a time; if you're constructing multiple clients with the same builder, you'll need to specify a new listener for each client. +Note that a `MicrometerApnsClientMetricsListener` is intended for use with only one `ApnsClient` at a time; if you're constructing multiple clients with the same builder, you'll need to specify a new listener for each client and should consider supplying identifying tags to each listener. ## License diff --git a/micrometer-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListener.java b/micrometer-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListener.java index 12c604e59..0153e2b67 100644 --- a/micrometer-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListener.java +++ b/micrometer-metrics-listener/src/main/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListener.java @@ -24,130 +24,144 @@ import com.eatthepath.pushy.apns.ApnsClient; import com.eatthepath.pushy.apns.ApnsClientMetricsListener; +import com.eatthepath.pushy.apns.PushNotificationResponse; import io.micrometer.core.instrument.*; -import java.util.List; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; /** - *

An {@link ApnsClientMetricsListener} implementation that gathers and reports metrics - * using the Micrometer application monitoring library. A - * {@code MicrometerApnsClientMetricsListener} is intended to be used with a single - * {@link ApnsClient} instance; to gather metrics from multiple clients, callers should create - * multiple listeners.

+ *

An {@link ApnsClientMetricsListener} implementation that gathers and reports metrics using the + * Micrometer application monitoring library. A + * {@code MicrometerApnsClientMetricsListener} is intended to be used with a single {@link ApnsClient} instance; to + * gather metrics from multiple clients, callers should create multiple listeners and should consider providing separate + * sets of identifying tags to each listener.

* *

Callers provide a {@link io.micrometer.core.instrument.MeterRegistry} to the * {@code MicrometerApnsClientMetricsListener} at construction time, and the listener populates the registry with the * following metrics:

* *
- *
{@value #NOTIFICATION_TIMER_NAME}
- *
A {@link io.micrometer.core.instrument.Timer} that measures the time between sending notifications and receiving - * a reply (whether accepted or rejected) from the APNs server.
- * *
{@value #WRITE_FAILURES_COUNTER_NAME}
- *
A {@link io.micrometer.core.instrument.Counter} that measures the number and rate of failures to send notifications to the - * APNs server.
+ *
A {@link io.micrometer.core.instrument.Counter} that measures the number of failures to send notifications to + * the APNs server. In addition to the tags provided at listener construction time, this counter will also be tagged + * with: + * + *
+ *
{@value #TOPIC_TAG_NAME}
+ *
The APNs topic to which each notification was sent
+ *
+ *
* *
{@value #SENT_NOTIFICATIONS_COUNTER_NAME}
- *
A {@link io.micrometer.core.instrument.Counter} that measures the number and rate of notifications successfully sent to the - * APNs server.
+ *
A {@link io.micrometer.core.instrument.Counter} that measures the number of notifications successfully sent to + * the APNs server. In addition to the tags provided at listener construction time, this counter will also be tagged + * with: + * + *
+ *
{@value #TOPIC_TAG_NAME}
+ *
The APNs topic to which each notification was sent
+ *
+ *
* - *
{@value #ACCEPTED_NOTIFICATIONS_COUNTER_NAME}
- *
A {@link io.micrometer.core.instrument.Counter} that measures the number and rate of notifications accepted by the APNs - * server.
+ *
{@value #ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME}
+ *
A {@link io.micrometer.core.instrument.Timer} that measures the time between sending notifications and receiving + * a reply (whether accepted or rejected) from the APNs server. In addition to the tags provided at listener + * construction time, this counter will also be tagged with: * - *
{@value #REJECTED_NOTIFICATIONS_COUNTER_NAME}
- *
A {@link io.micrometer.core.instrument.Counter} that measures the number and rate of notifications rejected by the APNs - * server.
+ *
+ *
{@value #TOPIC_TAG_NAME}
+ *
The APNs topic to which each notification was sent
+ * + *
{@value #ACCEPTED_TAG_NAME}
+ *
"true" if the notification was accepted by the APNs server or "false" if the notification was rejected
+ * + *
{@value #STATUS_TAG_NAME}
+ *
The HTTP status code returned by the APNs server
+ * + *
{@value REASON_TAG_NAME}
+ *
The rejection reason provided by the APNs server if the notification was rejected; not set if the notification + * was accepted
+ *
+ * * *
{@value #OPEN_CONNECTIONS_GAUGE_NAME}
*
A {@link io.micrometer.core.instrument.Gauge} that indicates number of open connections.
* *
{@value #CONNECTION_FAILURES_COUNTER_NAME}
- *
A {@link io.micrometer.core.instrument.Counter} that measures the number and rate of failed attempts to connect to the APNs - * server.
+ *
A {@link io.micrometer.core.instrument.Counter} that measures the number of failed attempts to connect to the + * APNs server.
*
* * @author Jon Chambers */ public class MicrometerApnsClientMetricsListener implements ApnsClientMetricsListener { - private final Timer notificationTimer; - private final ConcurrentMap notificationStartTimes; - - private final Counter writeFailures; - private final Counter sentNotifications; - private final Counter acceptedNotifications; - private final Counter rejectedNotifications; + private final MeterRegistry meterRegistry; + private final Tags tags; private final AtomicInteger openConnections = new AtomicInteger(0); private final Counter connectionFailures; - private static final String[] EMPTY_STRING_ARRAY = new String[0]; - - /** - * The name of a {@link io.micrometer.core.instrument.Timer} that measures round-trip time when sending - * notifications. - */ - public static final String NOTIFICATION_TIMER_NAME = "notifications.sent.timer"; - /** * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of write failures when * sending notifications. */ - public static final String WRITE_FAILURES_COUNTER_NAME = "notifications.failed"; + public static final String WRITE_FAILURES_COUNTER_NAME = "pushy.notifications.failed"; /** * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of notifications sent * (regardless of whether they're accepted or rejected by the server). */ - public static final String SENT_NOTIFICATIONS_COUNTER_NAME = "notifications.sent"; + public static final String SENT_NOTIFICATIONS_COUNTER_NAME = "pushy.notifications.sent"; /** - * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of notifications accepted by - * the APNs server. - */ - public static final String ACCEPTED_NOTIFICATIONS_COUNTER_NAME = "notifications.accepted"; - - /** - * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of notifications rejected by - * the APNs server. + * The name of a {@link io.micrometer.core.instrument.Timer} that measures round-trip time when sending + * notifications. */ - public static final String REJECTED_NOTIFICATIONS_COUNTER_NAME = "notifications.rejected"; + public static final String ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME = "pushy.notifications.acknowledged"; /** * The name of a {@link io.micrometer.core.instrument.Gauge} that measures the number of open connections in an APNs * client's internal connection pool. */ - public static final String OPEN_CONNECTIONS_GAUGE_NAME = "connections.open"; + public static final String OPEN_CONNECTIONS_GAUGE_NAME = "pushy.connections.open"; /** * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of a client's failed * connection attempts. */ - public static final String CONNECTION_FAILURES_COUNTER_NAME = "connections.failed"; + public static final String CONNECTION_FAILURES_COUNTER_NAME = "pushy.connections.failed"; /** - * Constructs a new Micrometer metrics listener that adds metrics to the given registry with the given list of tags. - * - * @param meterRegistry the registry to which to add metrics - * @param tags an optional list of tags to attach to metrics; may be {@code null} or empty, in which case no tags - * are added + * The name of a tag attached to most metrics that indicates the APNs topic to which a notification was sent. */ - public MicrometerApnsClientMetricsListener(final MeterRegistry meterRegistry, final List tags) { - this(meterRegistry, tags != null ? tags.toArray(EMPTY_STRING_ARRAY) : null); - } + public static final String TOPIC_TAG_NAME = "topic"; + + /** + * The name of a tag attached to the {@value #ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME} timer indicating if a + * notification was accepted by the APNs server. + */ + public static final String ACCEPTED_TAG_NAME = "accepted"; + + /** + * The name of a tag attached to the {@value #ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME} timer indicating the reason why + * a notification was rejected by the APNs server. + */ + public static final String REASON_TAG_NAME = "reason"; + + /** + * The name of a tag attached to the {@value #ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME} timer indicating the HTTP + * status code reported by the APNs server. + */ + public static final String STATUS_TAG_NAME = "status"; /** * Constructs a new Micrometer metrics listener that adds metrics to the given registry with the given list of tags. * * @param meterRegistry the registry to which to add metrics - * @param tagKeysAndValues an optional list of tag keys/values to attach to metrics; must be an even number of - * strings representing alternating key/value pairs + * @param tagKeysAndValues an optional list of tag keys/values to attach to all metrics produced by this listener; + * must be an even number of strings representing alternating key/value pairs */ public MicrometerApnsClientMetricsListener(final MeterRegistry meterRegistry, final String... tagKeysAndValues) { this(meterRegistry, Tags.of(tagKeysAndValues)); @@ -157,113 +171,82 @@ public MicrometerApnsClientMetricsListener(final MeterRegistry meterRegistry, fi * Constructs a new Micrometer metrics listener that adds metrics to the given registry with the given list of tags. * * @param meterRegistry the registry to which to add metrics - * @param tags an optional collection of tags to attach to metrics; may be empty + * @param tags an optional collection of tags to attach to all metrics produced by this listener; may be empty + * or {@code null} */ public MicrometerApnsClientMetricsListener(final MeterRegistry meterRegistry, final Iterable tags) { - this.notificationStartTimes = new ConcurrentHashMap<>(); - this.notificationTimer = meterRegistry.timer(NOTIFICATION_TIMER_NAME, tags); + this.meterRegistry = meterRegistry; + this.tags = Tags.of(tags); - this.writeFailures = meterRegistry.counter(WRITE_FAILURES_COUNTER_NAME, tags); - this.sentNotifications = meterRegistry.counter(SENT_NOTIFICATIONS_COUNTER_NAME, tags); - this.acceptedNotifications = meterRegistry.counter(ACCEPTED_NOTIFICATIONS_COUNTER_NAME, tags); - this.rejectedNotifications = meterRegistry.counter(REJECTED_NOTIFICATIONS_COUNTER_NAME, tags); - - this.connectionFailures = meterRegistry.counter(CONNECTION_FAILURES_COUNTER_NAME, tags); - - meterRegistry.gauge(OPEN_CONNECTIONS_GAUGE_NAME, tags, openConnections); + this.connectionFailures = meterRegistry.counter(CONNECTION_FAILURES_COUNTER_NAME, this.tags); + meterRegistry.gauge(OPEN_CONNECTIONS_GAUGE_NAME, this.tags, openConnections); } /** * Records a failed attempt to send a notification and updates metrics accordingly. * - * @param apnsClient the client that failed to write the notification; note that this is ignored by - * {@code MicrometerApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that could not be written + * @param topic the APNs topic to which the notification was sent */ @Override - public void handleWriteFailure(final ApnsClient apnsClient, final long notificationId) { - this.notificationStartTimes.remove(notificationId); - this.writeFailures.increment(); + public void handleWriteFailure(final String topic) { + this.meterRegistry.counter(WRITE_FAILURES_COUNTER_NAME, this.tags.and(TOPIC_TAG_NAME, topic)).increment(); } /** * Records a successful attempt to send a notification and updates metrics accordingly. * - * @param apnsClient the client that sent the notification; note that this is ignored by - * {@code MicrometerApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that was sent + * @param topic the APNs topic to which the notification was sent */ @Override - public void handleNotificationSent(final ApnsClient apnsClient, final long notificationId) { - this.notificationStartTimes.put(notificationId, System.nanoTime()); - this.sentNotifications.increment(); + public void handleNotificationSent(final String topic) { + this.meterRegistry.counter(SENT_NOTIFICATIONS_COUNTER_NAME, this.tags.and(TOPIC_TAG_NAME, topic)).increment(); } /** - * Records that the APNs server accepted a previously-sent notification and updates metrics accordingly. + * Records that the APNs server accepted or rejected a previously-sent notification and updates metrics accordingly. * - * @param apnsClient the client that sent the accepted notification; note that this is ignored by - * {@code MicrometerApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that was accepted + * @param response the response from the APNs server + * @param durationNanos the duration, in nanoseconds, between the time the notification was initially sent and when + * it was acknowledged by the APNs server */ @Override - public void handleNotificationAccepted(final ApnsClient apnsClient, final long notificationId) { - this.recordEndTimeForNotification(notificationId); - this.acceptedNotifications.increment(); - } - - /** - * Records that the APNs server rejected a previously-sent notification and updates metrics accordingly. - * - * @param apnsClient the client that sent the rejected notification; note that this is ignored by - * {@code MicrometerApnsClientMetricsListener} instances, which should always be used for exactly one client - * @param notificationId an opaque, unique identifier for the notification that was rejected - */ - @Override - public void handleNotificationRejected(final ApnsClient apnsClient, final long notificationId) { - this.recordEndTimeForNotification(notificationId); - this.rejectedNotifications.increment(); - } - - private void recordEndTimeForNotification(final long notificationId) { - final long endTime = System.nanoTime(); - final Long startTime = this.notificationStartTimes.remove(notificationId); - - if (startTime != null) { - this.notificationTimer.record(endTime - startTime, TimeUnit.NANOSECONDS); + public void handleNotificationAcknowledged(final PushNotificationResponse response, final long durationNanos) { + Tags tags = this.tags.and( + TOPIC_TAG_NAME, response.getPushNotification().getTopic(), + ACCEPTED_TAG_NAME, String.valueOf(response.isAccepted()), + STATUS_TAG_NAME, String.valueOf(response.getStatusCode()) + ); + + if (!response.isAccepted()) { + tags = tags.and(REASON_TAG_NAME, response.getRejectionReason().orElse("unknown")); } + + this.meterRegistry.timer(ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME, tags).record(durationNanos, TimeUnit.NANOSECONDS); } /** * Records that the APNs server added a new connection to its internal connection pool and updates metrics * accordingly. - * - * @param apnsClient the client that added the new connection */ @Override - public void handleConnectionAdded(final ApnsClient apnsClient) { + public void handleConnectionAdded() { this.openConnections.incrementAndGet(); } /** * Records that the APNs server removed a connection from its internal connection pool and updates metrics * accordingly. - * - * @param apnsClient the client that removed the connection */ @Override - public void handleConnectionRemoved(final ApnsClient apnsClient) { + public void handleConnectionRemoved() { this.openConnections.decrementAndGet(); } /** * Records that a previously-started attempt to connect to the APNs server failed and updates metrics accordingly. - * - * @param apnsClient the client that failed to connect; note that this is ignored by - * {@code MicrometerApnsClientMetricsListener} instances, which should always be used for exactly one client */ @Override - public void handleConnectionCreationFailed(final ApnsClient apnsClient) { + public void handleConnectionCreationFailed() { this.connectionFailures.increment(); } } diff --git a/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/ExampleApp.java b/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/ExampleApp.java index 58f4afe71..b184406f2 100644 --- a/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/ExampleApp.java +++ b/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/ExampleApp.java @@ -40,8 +40,7 @@ public static void main(final String... args) throws Exception { // clients with the same builder, you'll need to specify a new listener // for each client. final MicrometerApnsClientMetricsListener listener = - new MicrometerApnsClientMetricsListener(existingMeterRegistry, - "notifications", "apns", "pushy"); + new MicrometerApnsClientMetricsListener(existingMeterRegistry); final ApnsClient apnsClient = new ApnsClientBuilder() .setApnsServer(ApnsClientBuilder.DEVELOPMENT_APNS_HOST) diff --git a/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListenerTest.java b/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListenerTest.java index ebfd31ed0..b6df3bcea 100644 --- a/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListenerTest.java +++ b/micrometer-metrics-listener/src/test/java/com/eatthepath/pushy/apns/metrics/micrometer/MicrometerApnsClientMetricsListenerTest.java @@ -22,16 +22,20 @@ package com.eatthepath.pushy.apns.metrics.micrometer; +import com.eatthepath.pushy.apns.ApnsPushNotification; +import com.eatthepath.pushy.apns.PushNotificationResponse; +import com.eatthepath.pushy.apns.util.SimpleApnsPushNotification; import io.micrometer.core.instrument.*; import io.micrometer.core.instrument.simple.SimpleMeterRegistry; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.util.Collections; +import java.time.Instant; import java.util.List; +import java.util.Optional; +import java.util.UUID; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; public class MicrometerApnsClientMetricsListenerTest { @@ -46,91 +50,113 @@ public void setUp() { } @Test - public void testMicrometerApnsClientMetricsListenerNoTags() { - for (final Meter meter : this.meterRegistry.getMeters()) { - assertTrue(meter.getId().getTags().isEmpty()); - } - } - - @Test - public void testMicrometerApnsClientMetricsListenerVariadicTags() { - final MeterRegistry taggedMeterRegistry = new SimpleMeterRegistry(); - new MicrometerApnsClientMetricsListener(taggedMeterRegistry, "key", "value"); - - final List expectedTags = Collections.singletonList(Tag.of("key", "value")); - - for (final Meter meter : taggedMeterRegistry.getMeters()) { - assertEquals(expectedTags, meter.getId().getTags()); - } - } - - @Test - public void testMicrometerApnsClientMetricsListenerIterableTags() { - final MeterRegistry taggedMeterRegistry = new SimpleMeterRegistry(); - final List tags = Collections.singletonList(Tag.of("key", "value")); - - new MicrometerApnsClientMetricsListener(taggedMeterRegistry, tags); - - for (final Meter meter : taggedMeterRegistry.getMeters()) { - assertEquals(tags, meter.getId().getTags()); - } + public void testMicrometerApnsClientMetricsListener() { + assertDoesNotThrow(() -> new MicrometerApnsClientMetricsListener(new SimpleMeterRegistry())); + assertDoesNotThrow(() -> new MicrometerApnsClientMetricsListener(new SimpleMeterRegistry(), "tag", "value")); + assertDoesNotThrow(() -> new MicrometerApnsClientMetricsListener(new SimpleMeterRegistry(), Tags.of("tag", "value"))); + assertDoesNotThrow(() -> new MicrometerApnsClientMetricsListener(new SimpleMeterRegistry(), (List) null)); } @Test public void testHandleWriteFailure() { - final Counter writeFailures = this.meterRegistry.get(MicrometerApnsClientMetricsListener.WRITE_FAILURES_COUNTER_NAME).counter(); - assertEquals(0, (int) writeFailures.count()); - - this.listener.handleWriteFailure(null, 1); - assertEquals(1, (int) writeFailures.count()); + this.listener.handleWriteFailure("com.example.topic"); + assertEquals(1, (int) this.meterRegistry.get(MicrometerApnsClientMetricsListener.WRITE_FAILURES_COUNTER_NAME).counter().count()); } @Test public void testHandleNotificationSent() { - final Counter sentNotifications = this.meterRegistry.get(MicrometerApnsClientMetricsListener.SENT_NOTIFICATIONS_COUNTER_NAME).counter(); - assertEquals(0, (int) sentNotifications.count()); - - this.listener.handleNotificationSent(null, 1); - assertEquals(1, (int) sentNotifications.count()); + this.listener.handleNotificationSent("com.example.topic"); + assertEquals(1, (int) this.meterRegistry.get(MicrometerApnsClientMetricsListener.SENT_NOTIFICATIONS_COUNTER_NAME).counter().count()); } @Test public void testHandleNotificationAccepted() { - final Counter acceptedNotifications = this.meterRegistry.get(MicrometerApnsClientMetricsListener.ACCEPTED_NOTIFICATIONS_COUNTER_NAME).counter(); - assertEquals(0, (int) acceptedNotifications.count()); + final String topic = "com.example.topic"; + final int status = 200; + + this.listener.handleNotificationAcknowledged(buildPushNotificationResponse(topic, true, status, null), 1); + + final Timer acknowledgedNotifications = this.meterRegistry + .get(MicrometerApnsClientMetricsListener.ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME) + .tags(MicrometerApnsClientMetricsListener.TOPIC_TAG_NAME, topic, + MicrometerApnsClientMetricsListener.ACCEPTED_TAG_NAME, String.valueOf(true), + MicrometerApnsClientMetricsListener.STATUS_TAG_NAME, String.valueOf(status)) + .timer(); - this.listener.handleNotificationAccepted(null, 1); - assertEquals(1, (int) acceptedNotifications.count()); + assertEquals(1, (int) acknowledgedNotifications.count()); } @Test public void testHandleNotificationRejected() { - final Counter rejectedNotifications = this.meterRegistry.get(MicrometerApnsClientMetricsListener.REJECTED_NOTIFICATIONS_COUNTER_NAME).counter(); - assertEquals(0, (int) rejectedNotifications.count()); + final String topic = "com.example.topic"; + final int status = 400; + final String rejectionReason = "BadDeviceToken"; - this.listener.handleNotificationRejected(null, 1); - assertEquals(1, (int) rejectedNotifications.count()); + this.listener.handleNotificationAcknowledged(buildPushNotificationResponse(topic, false, status, rejectionReason), 1); + + final Timer acknowledgedNotifications = this.meterRegistry + .get(MicrometerApnsClientMetricsListener.ACKNOWLEDGED_NOTIFICATIONS_TIMER_NAME) + .tags(MicrometerApnsClientMetricsListener.TOPIC_TAG_NAME, topic, + MicrometerApnsClientMetricsListener.ACCEPTED_TAG_NAME, String.valueOf(false), + MicrometerApnsClientMetricsListener.STATUS_TAG_NAME, String.valueOf(status), + MicrometerApnsClientMetricsListener.REASON_TAG_NAME, rejectionReason) + .timer(); + + assertEquals(1, (int) acknowledgedNotifications.count()); } @Test public void testHandleConnectionAddedAndRemoved() { - final Gauge openConnectionGauge = this.meterRegistry.get(MicrometerApnsClientMetricsListener.OPEN_CONNECTIONS_GAUGE_NAME).gauge(); + this.listener.handleConnectionAdded(); - this.listener.handleConnectionAdded(null); + assertEquals(1, (int) this.meterRegistry.get(MicrometerApnsClientMetricsListener.OPEN_CONNECTIONS_GAUGE_NAME).gauge().value()); - assertEquals(1, (int) openConnectionGauge.value()); + this.listener.handleConnectionRemoved(); - this.listener.handleConnectionRemoved(null); - - assertEquals(0, (int) openConnectionGauge.value()); + assertEquals(0, (int) this.meterRegistry.get(MicrometerApnsClientMetricsListener.OPEN_CONNECTIONS_GAUGE_NAME).gauge().value()); } @Test public void testHandleConnectionCreationFailed() { - final Counter connectionFailures = this.meterRegistry.get(MicrometerApnsClientMetricsListener.CONNECTION_FAILURES_COUNTER_NAME).counter(); - assertEquals(0, (int) connectionFailures.count()); + this.listener.handleConnectionCreationFailed(); + assertEquals(1, (int) this.meterRegistry.get(MicrometerApnsClientMetricsListener.CONNECTION_FAILURES_COUNTER_NAME).counter().count()); + } - this.listener.handleConnectionCreationFailed(null); - assertEquals(1, (int) connectionFailures.count()); + private static PushNotificationResponse buildPushNotificationResponse(final String topic, + final boolean accepted, + final int status, + final String rejectionReason) { + + return new PushNotificationResponse() { + @Override + public ApnsPushNotification getPushNotification() { + return new SimpleApnsPushNotification("device-token", topic, "{}"); + } + + @Override + public boolean isAccepted() { + return accepted; + } + + @Override + public UUID getApnsId() { + return null; + } + + @Override + public int getStatusCode() { + return status; + } + + @Override + public Optional getRejectionReason() { + return Optional.ofNullable(rejectionReason); + } + + @Override + public Optional getTokenInvalidationTimestamp() { + return Optional.empty(); + } + }; } } diff --git a/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClient.java b/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClient.java index cba284ef9..13c897732 100644 --- a/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClient.java +++ b/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClient.java @@ -34,7 +34,6 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; /** *

An APNs client sends push notifications to the APNs gateway. Clients authenticate themselves to APNs servers in @@ -82,7 +81,6 @@ public class ApnsClient { private final ApnsChannelPool channelPool; private final ApnsClientMetricsListener metricsListener; - private final AtomicLong nextNotificationId = new AtomicLong(0); private final AtomicBoolean isClosed = new AtomicBoolean(false); @@ -93,32 +91,29 @@ public class ApnsClient { private static class NoopApnsClientMetricsListener implements ApnsClientMetricsListener { - @Override - public void handleWriteFailure(final ApnsClient apnsClient, final long notificationId) { - } @Override - public void handleNotificationSent(final ApnsClient apnsClient, final long notificationId) { + public void handleWriteFailure(final String topic) { } @Override - public void handleNotificationAccepted(final ApnsClient apnsClient, final long notificationId) { + public void handleNotificationSent(final String topic) { } @Override - public void handleNotificationRejected(final ApnsClient apnsClient, final long notificationId) { + public void handleNotificationAcknowledged(final PushNotificationResponse response, final long durationNanos) { } @Override - public void handleConnectionAdded(final ApnsClient apnsClient) { + public void handleConnectionAdded() { } @Override - public void handleConnectionRemoved(final ApnsClient apnsClient) { + public void handleConnectionRemoved() { } @Override - public void handleConnectionCreationFailed(final ApnsClient apnsClient) { + public void handleConnectionCreationFailed() { } } @@ -142,17 +137,17 @@ protected ApnsClient(final ApnsClientConfiguration clientConfiguration, final Ev @Override public void handleConnectionAdded() { - ApnsClient.this.metricsListener.handleConnectionAdded(ApnsClient.this); + ApnsClient.this.metricsListener.handleConnectionAdded(); } @Override public void handleConnectionRemoved() { - ApnsClient.this.metricsListener.handleConnectionRemoved(ApnsClient.this); + ApnsClient.this.metricsListener.handleConnectionRemoved(); } @Override public void handleConnectionCreationFailed() { - ApnsClient.this.metricsListener.handleConnectionCreationFailed(ApnsClient.this); + ApnsClient.this.metricsListener.handleConnectionCreationFailed(); } }; @@ -186,7 +181,7 @@ public PushNotificationFuture(notification); if (!this.isClosed.get()) { - final long notificationId = this.nextNotificationId.getAndIncrement(); + final long start = System.nanoTime(); this.channelPool.acquire().addListener((GenericFutureListener>) acquireFuture -> { if (acquireFuture.isSuccess()) { @@ -194,7 +189,7 @@ public PushNotificationFuture) future -> { if (future.isSuccess()) { - ApnsClient.this.metricsListener.handleNotificationSent(ApnsClient.this, notificationId); + ApnsClient.this.metricsListener.handleNotificationSent(notification.getTopic()); } }); @@ -205,14 +200,12 @@ public PushNotificationFuture { + final long end = System.nanoTime(); + if (response != null) { - if (response.isAccepted()) { - ApnsClient.this.metricsListener.handleNotificationAccepted(ApnsClient.this, notificationId); - } else { - ApnsClient.this.metricsListener.handleNotificationRejected(ApnsClient.this, notificationId); - } + ApnsClient.this.metricsListener.handleNotificationAcknowledged(response, end - start); } else { - ApnsClient.this.metricsListener.handleWriteFailure(ApnsClient.this, notificationId); + ApnsClient.this.metricsListener.handleWriteFailure(notification.getTopic()); } }); } else { diff --git a/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClientMetricsListener.java b/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClientMetricsListener.java index f65d9b1db..7ce9d9caa 100644 --- a/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClientMetricsListener.java +++ b/pushy/src/main/java/com/eatthepath/pushy/apns/ApnsClientMetricsListener.java @@ -42,79 +42,60 @@ public interface ApnsClientMetricsListener { /** - * Indicates that an attempt to send a push notification failed before the notification was processed by the APNs - * server. Write failures may be the first event in a sequence for a given notification ID (indicating that the + * Indicates that an attempt to send a push notification failed before the notification was acknowledged by the APNs + * server. Write failures may be the first event in a sequence for a given notification (indicating that the * notification was never written to the wire), but may also occur after a notification was sent if the connection * closed before the notification was acknowledged by the server. * - * @param apnsClient the client that sent the notification - * @param notificationId an opaque identifier for the push notification that can be used to correlate this event - * with other events related to the same notification + * @param topic the APNs topic to which the notification was sent * - * @since 0.6 + * @since 0.16 */ - void handleWriteFailure(ApnsClient apnsClient, long notificationId); + void handleWriteFailure(String topic); /** * Indicates that a notification was sent to the APNs server. Note that a sent notification may still be either * accepted or rejected by the APNs server later; sending the notification doesn't imply anything about the ultimate * state of the notification. * - * @param apnsClient the client that sent the notification - * @param notificationId an opaque identifier for the push notification that can be used to correlate this event - * with other events related to the same notification + * @param topic the APNs topic to which the notification was sent * - * @since 0.6 + * @since 0.16 */ - void handleNotificationSent(ApnsClient apnsClient, long notificationId); + void handleNotificationSent(String topic); /** - * Indicates that a notification that was previously sent to an APNs server was accepted by the server. + * Indicates that a notification that was previously sent to an APNs server was acknowledged (i.e. either accepted + * or rejected) by the server. * - * @param apnsClient the client that sent the notification - * @param notificationId an opaque identifier for the push notification that can be used to correlate this event - * with other events related to the same notification + * @param response the response from the server + * @param durationNanos the duration, measured in nanoseconds, between the time when + * {@link ApnsClient#sendNotification(ApnsPushNotification)} was called for the notification in question and when + * the server acknowledged the notification * - * @since 0.6 + * @since 0.16 */ - void handleNotificationAccepted(ApnsClient apnsClient, long notificationId); - - /** - * Indicates that a notification that was previously sent to an APNs server was rejected by the server. - * - * @param apnsClient the client that sent the notification - * @param notificationId an opaque identifier for the push notification that can be used to correlate this event - * with other events related to the same notification - * - * @since 0.6 - */ - void handleNotificationRejected(ApnsClient apnsClient, long notificationId); + void handleNotificationAcknowledged(PushNotificationResponse response, long durationNanos); /** * Indicates that the client has successfully created a new connection to the APNs server in its internal * connection pool. * - * @param apnsClient the client that created the new connection - * * @since 0.11 */ - void handleConnectionAdded(ApnsClient apnsClient); + void handleConnectionAdded(); /** * Indicates that the client has removed a previously-added connection from its internal connection pool. * - * @param apnsClient the client that removed the connection - * * @since 0.11 */ - void handleConnectionRemoved(ApnsClient apnsClient); + void handleConnectionRemoved(); /** * Indicates that an attempt to create a new connection to the APNs server failed. * - * @param apnsClient the client that attempted to create a new connection - * * @since 0.11 */ - void handleConnectionCreationFailed(ApnsClient apnsClient); + void handleConnectionCreationFailed(); } diff --git a/pushy/src/test/java/com/eatthepath/pushy/apns/ApnsClientTest.java b/pushy/src/test/java/com/eatthepath/pushy/apns/ApnsClientTest.java index 9afdafaf7..7ad92a846 100644 --- a/pushy/src/test/java/com/eatthepath/pushy/apns/ApnsClientTest.java +++ b/pushy/src/test/java/com/eatthepath/pushy/apns/ApnsClientTest.java @@ -51,46 +51,48 @@ public class ApnsClientTest extends AbstractClientServerTest { private static class TestClientMetricsListener implements ApnsClientMetricsListener { - private final List writeFailures = new ArrayList<>(); - private final List sentNotifications = new ArrayList<>(); - private final List acceptedNotifications = new ArrayList<>(); - private final List rejectedNotifications = new ArrayList<>(); + private final AtomicInteger writeFailures = new AtomicInteger(0); + private final AtomicInteger sentNotifications = new AtomicInteger(0); + private final AtomicInteger acceptedNotifications = new AtomicInteger(0); + private final AtomicInteger rejectedNotifications = new AtomicInteger(0); private final AtomicInteger connectionsAdded = new AtomicInteger(0); private final AtomicInteger connectionsRemoved = new AtomicInteger(0); private final AtomicInteger failedConnectionAttempts = new AtomicInteger(0); @Override - public void handleWriteFailure(final ApnsClient apnsClient, final long notificationId) { + public void handleWriteFailure(final String topic) { synchronized (this.writeFailures) { - this.writeFailures.add(notificationId); + this.writeFailures.incrementAndGet(); this.writeFailures.notifyAll(); } } @Override - public void handleNotificationSent(final ApnsClient apnsClient, final long notificationId) { - this.sentNotifications.add(notificationId); - } - - @Override - public void handleNotificationAccepted(final ApnsClient apnsClient, final long notificationId) { - synchronized (this.acceptedNotifications) { - this.acceptedNotifications.add(notificationId); - this.acceptedNotifications.notifyAll(); + public void handleNotificationSent(final String topic) { + synchronized (this.sentNotifications) { + this.sentNotifications.incrementAndGet(); + this.sentNotifications.notifyAll(); } } @Override - public void handleNotificationRejected(final ApnsClient apnsClient, final long notificationId) { - synchronized (this.rejectedNotifications) { - this.rejectedNotifications.add(notificationId); - this.rejectedNotifications.notifyAll(); + public void handleNotificationAcknowledged(final PushNotificationResponse response, final long durationNanos) { + if (response.isAccepted()) { + synchronized (this.acceptedNotifications) { + this.acceptedNotifications.incrementAndGet(); + this.acceptedNotifications.notifyAll(); + } + } else { + synchronized (this.rejectedNotifications) { + this.rejectedNotifications.incrementAndGet(); + this.rejectedNotifications.notifyAll(); + } } } @Override - public void handleConnectionAdded(final ApnsClient apnsClient) { + public void handleConnectionAdded() { synchronized (this.connectionsAdded) { this.connectionsAdded.incrementAndGet(); this.connectionsAdded.notifyAll(); @@ -98,7 +100,7 @@ public void handleConnectionAdded(final ApnsClient apnsClient) { } @Override - public void handleConnectionRemoved(final ApnsClient apnsClient) { + public void handleConnectionRemoved() { synchronized (this.connectionsRemoved) { this.connectionsRemoved.incrementAndGet(); this.connectionsRemoved.notifyAll(); @@ -106,7 +108,7 @@ public void handleConnectionRemoved(final ApnsClient apnsClient) { } @Override - public void handleConnectionCreationFailed(final ApnsClient apnsClient) { + public void handleConnectionCreationFailed() { synchronized (this.failedConnectionAttempts) { this.failedConnectionAttempts.incrementAndGet(); this.failedConnectionAttempts.notifyAll(); @@ -115,7 +117,7 @@ public void handleConnectionCreationFailed(final ApnsClient apnsClient) { void waitForNonZeroWriteFailures() throws InterruptedException { synchronized (this.writeFailures) { - while (this.writeFailures.isEmpty()) { + while (this.writeFailures.get() == 0) { this.writeFailures.wait(); } } @@ -123,7 +125,7 @@ void waitForNonZeroWriteFailures() throws InterruptedException { void waitForNonZeroAcceptedNotifications() throws InterruptedException { synchronized (this.acceptedNotifications) { - while (this.acceptedNotifications.isEmpty()) { + while (this.acceptedNotifications.get() == 0) { this.acceptedNotifications.wait(); } } @@ -131,7 +133,7 @@ void waitForNonZeroAcceptedNotifications() throws InterruptedException { void waitForNonZeroRejectedNotifications() throws InterruptedException { synchronized (this.rejectedNotifications) { - while (this.rejectedNotifications.isEmpty()) { + while (this.rejectedNotifications.get() == 0) { this.rejectedNotifications.wait(); } } @@ -145,19 +147,19 @@ void waitForNonZeroFailedConnections() throws InterruptedException { } } - List getWriteFailures() { + AtomicInteger getWriteFailures() { return this.writeFailures; } - List getSentNotifications() { + AtomicInteger getSentNotifications() { return this.sentNotifications; } - List getAcceptedNotifications() { + AtomicInteger getAcceptedNotifications() { return this.acceptedNotifications; } - List getRejectedNotifications() { + AtomicInteger getRejectedNotifications() { return this.rejectedNotifications; } @@ -415,9 +417,7 @@ void testSendNotificationWithExpiredAuthenticationToken() throws Exception { }; final MockApnsServer server = this.buildServer(expireFirstTokenHandlerFactory); - - final TestClientMetricsListener metricsListener = new TestClientMetricsListener(); - final ApnsClient client = this.buildTokenAuthenticationClient(metricsListener); + final ApnsClient client = this.buildTokenAuthenticationClient(); try { server.start(PORT).get(); @@ -586,9 +586,9 @@ void testAcceptedNotificationAndAddedConnectionMetrics(final boolean useTokenAut metricsListener.waitForNonZeroAcceptedNotifications(); - assertEquals(1, metricsListener.getSentNotifications().size()); - assertEquals(metricsListener.getSentNotifications(), metricsListener.getAcceptedNotifications()); - assertTrue(metricsListener.getRejectedNotifications().isEmpty()); + assertEquals(1, metricsListener.getSentNotifications().get()); + assertEquals(metricsListener.getSentNotifications().get(), metricsListener.getAcceptedNotifications().get()); + assertEquals(0, metricsListener.getRejectedNotifications().get()); assertEquals(1, metricsListener.getConnectionsAdded().get()); assertEquals(0, metricsListener.getConnectionsRemoved().get()); @@ -622,9 +622,9 @@ void testRejectedNotificationMetrics(final boolean useTokenAuthentication) throw metricsListener.waitForNonZeroRejectedNotifications(); - assertEquals(1, metricsListener.getSentNotifications().size()); - assertEquals(metricsListener.getSentNotifications(), metricsListener.getRejectedNotifications()); - assertTrue(metricsListener.getAcceptedNotifications().isEmpty()); + assertEquals(1, metricsListener.getSentNotifications().get()); + assertEquals(metricsListener.getSentNotifications().get(), metricsListener.getRejectedNotifications().get()); + assertEquals(0, metricsListener.getAcceptedNotifications().get()); } finally { client.close().get(); server.shutdown().get(); @@ -653,7 +653,7 @@ void testFailedConnectionAndWriteFailureMetrics(final boolean useTokenAuthentica assertEquals(0, metricsListener.getConnectionsRemoved().get()); assertEquals(1, metricsListener.getFailedConnectionAttempts().get()); - assertEquals(1, metricsListener.getWriteFailures().size()); + assertEquals(1, metricsListener.getWriteFailures().get()); } finally { client.close().get(); }