Skip to content
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.

Sign up for GitHub

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

Significantly overhaul the ApnsClientMetricsListener interface #1046

Merged
merged 1 commit into from
Jul 6, 2024
Merged

Significantly overhaul the ApnsClientMetricsListener interface #1046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 20 additions & 51 deletions ...ava/com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -78,7 +78,6 @@ public class DropwizardApnsClientMetricsListener implements ApnsClientMetricsLis
private final MetricRegistry metrics;

private final Timer notificationTimer;
private final ConcurrentMap<Long, Timer.Context> notificationTimerContexts;

private final Meter writeFailures;
private final Meter sentNotifications;
Expand Down Expand Up @@ -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);
Expand All @@ -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.
*
Expand Down
53 changes: 46 additions & 7 deletions ...com/eatthepath/pushy/apns/metrics/dropwizard/DropwizardApnsClientMetricsListenerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}

Expand All @@ -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());
}

Expand All @@ -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());
}

Expand All @@ -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());
}

Expand All @@ -84,11 +89,11 @@ public void testHandleConnectionAddedAndRemoved() {
@SuppressWarnings("unchecked")
final Gauge<Integer> openConnectionGauge = (Gauge<Integer>) 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());
}
Expand All @@ -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());
}

Expand All @@ -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<ApnsPushNotification>() {
@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<String> getRejectionReason() {
return Optional.empty();
}

@Override
public Optional<Instant> getTokenInvalidationTimestamp() {
return Optional.empty();
}
};
}
}
5 changes: 2 additions & 3 deletions micrometer-metrics-listener/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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

Expand Down
Loading