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

fix(snapshot): Cleanup notification handling for snapshots #917

Merged
merged 14 commits into from
Apr 29, 2022
Merged

fix(snapshot): Cleanup notification handling for snapshots #917

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
17e1dd7
Issue snapshot creation notification once the snapshot has passed ver…
Apr 27, 2022
4d3ba5c
fixup! Issue snapshot creation notification once the snapshot has pas…
Apr 27, 2022
8a2ba2e
fixup! Issue snapshot creation notification once the snapshot has pas…
Apr 28, 2022
37032ed
Add notifications for snapshot creation and deletion
Apr 28, 2022
5b3e9d1
Add regex to determine whether a recording up for deletion is a snaps…
Apr 29, 2022
a8be83c
Run spotless
Apr 29, 2022
17eb623
Extract determining whether a recording is a snapshot or not into the…
Apr 29, 2022
3dcab10
fixup! Extract determining whether a recording is a snapshot or not i…
Apr 29, 2022
ba3c4c8
Simplify notification handling for deleted recordings
Apr 29, 2022
f00f4ae
fixup! Simplify notification handling for deleted recordings
Apr 29, 2022
0151e16
Finish updating tests
Apr 29, 2022
9f3f3ab
Run spotless
Apr 29, 2022
35d1d93
Add more snapshot name test strings
Apr 29, 2022
c7e6eff
Bump web client commit
Apr 29, 2022
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
4 changes: 3 additions & 1 deletion src/main/java/io/cryostat/net/web/http/api/v1/TargetSnapshotPostHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
boolean verificationSuccessful = false;
try {
verificationSuccessful =
recordingTargetHelper.verifySnapshot(connectionDescriptor, snapshotName).get();
recordingTargetHelper
.verifySnapshot(connectionDescriptor, snapshotDescriptor)
.get();
} catch (ExecutionException e) {
handleExecutionException(e);
}
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/io/cryostat/net/web/http/api/v2/TargetSnapshotPostHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,9 @@ public IntermediateResponse<HyperlinkedSerializableRecordingDescriptor> handle(
boolean verificationSuccessful = false;
try {
verificationSuccessful =
recordingTargetHelper.verifySnapshot(connectionDescriptor, snapshotName).get();
recordingTargetHelper
.verifySnapshot(connectionDescriptor, snapshotDescriptor)
.get();
} catch (ExecutionException e) {
handleExecutionException(e);
}
Expand Down
68 changes: 41 additions & 27 deletions src/main/java/io/cryostat/recordings/RecordingTargetHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,19 @@

public class RecordingTargetHelper {

private static final String CREATE_NOTIFICATION_CATEGORY = "ActiveRecordingCreated";
private static final String CREATION_NOTIFICATION_CATEGORY = "ActiveRecordingCreated";
private static final String STOP_NOTIFICATION_CATEGORY = "ActiveRecordingStopped";
private static final String DELETION_NOTIFICATION_CATEGORY = "ActiveRecordingDeleted";
private static final String SNAPSHOT_CREATION_NOTIFICATION_CATEGORY = "SnapshotCreated";
private static final String SNAPSHOT_DELETION_NOTIFICATION_CATEGORY = "SnapshotDeleted";

private static final long TIMESTAMP_DRIFT_SAFEGUARD = 3_000L;

private static final Pattern TEMPLATE_PATTERN =
Pattern.compile("^template=([\\w]+)(?:,type=([\\w]+))?$");

private static final Pattern SNAPSHOT_NAME_PATTERN = Pattern.compile("^(snapshot\\-)([0-9]+)$");

private final TargetConnectionManager targetConnectionManager;
private final Lazy<WebServer> webServer;
private final EventOptionsBuilder.Factory eventOptionsBuilderFactory;
Expand Down Expand Up @@ -181,13 +186,7 @@ public IRecordingDescriptor startRecording(
webServer.get().getDownloadURL(connection, desc.getName()),
webServer.get().getReportURL(connection, desc.getName()),
metadata);
notificationFactory
.createBuilder()
.metaCategory(CREATE_NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.message(Map.of("recording", linkedDesc, "target", targetId))
.build()
.send();
this.issueNotification(targetId, linkedDesc, CREATION_NOTIFICATION_CATEGORY);

Object fixedDuration =
recordingOptions.get(RecordingOptionsBuilder.KEY_DURATION);
Expand Down Expand Up @@ -284,7 +283,7 @@ public IRecordingDescriptor stopRecording(
d,
webServer.get().getDownloadURL(connection, d.getName()),
webServer.get().getReportURL(connection, d.getName()));
this.notifyRecordingStopped(targetId, linkedDesc);
this.issueNotification(targetId, linkedDesc, STOP_NOTIFICATION_CATEGORY);
return getDescriptorByName(connection, recordingName).get();
} else {
throw new RecordingNotFoundException(targetId, recordingName);
Expand Down Expand Up @@ -346,9 +345,18 @@ public Future<HyperlinkedSerializableRecordingDescriptor> createSnapshot(
}

public Future<Boolean> verifySnapshot(
ConnectionDescriptor connectionDescriptor, String snapshotName) {
ConnectionDescriptor connectionDescriptor,
HyperlinkedSerializableRecordingDescriptor snapshotDescriptor) {
return this.verifySnapshot(connectionDescriptor, snapshotDescriptor, true);
}

public Future<Boolean> verifySnapshot(
ConnectionDescriptor connectionDescriptor,
HyperlinkedSerializableRecordingDescriptor snapshotDescriptor,
boolean issueNotification) {
CompletableFuture<Boolean> future = new CompletableFuture<>();
try {
String snapshotName = snapshotDescriptor.getName();
Optional<InputStream> snapshotOptional =
this.getRecording(connectionDescriptor, snapshotName).get();
if (snapshotOptional.isEmpty()) {
Expand All @@ -360,6 +368,12 @@ public Future<Boolean> verifySnapshot(
this.deleteRecording(connectionDescriptor, snapshotName, false).get();
future.complete(false);
} else {
if (issueNotification) {
this.issueNotification(
connectionDescriptor.getTargetId(),
snapshotDescriptor,
SNAPSHOT_CREATION_NOTIFICATION_CATEGORY);
}
future.complete(true);
}
}
Expand Down Expand Up @@ -428,18 +442,13 @@ private Future<Void> deleteRecording(
recordingMetadataManager.deleteRecordingMetadataIfExists(
connectionDescriptor.getTargetId(), recordingName);
if (issueNotification) {
notificationFactory
.createBuilder()
.metaCategory(DELETION_NOTIFICATION_CATEGORY)
.metaType(HttpMimeType.JSON)
.message(
Map.of(
"recording",
linkedDesc,
"target",
connectionDescriptor.getTargetId()))
.build()
.send();
Matcher m = SNAPSHOT_NAME_PATTERN.matcher(recordingName);
String notificationCategory =
m.matches()
? SNAPSHOT_DELETION_NOTIFICATION_CATEGORY
: DELETION_NOTIFICATION_CATEGORY;
this.issueNotification(
targetId, linkedDesc, notificationCategory);
}
} else {
throw new RecordingNotFoundException(targetId, recordingName);
Expand All @@ -453,13 +462,15 @@ private Future<Void> deleteRecording(
return future;
}

private void notifyRecordingStopped(
String targetId, HyperlinkedSerializableRecordingDescriptor desc) {
private void issueNotification(
String targetId,
HyperlinkedSerializableRecordingDescriptor linkedDesc,
String notificationCategory) {
notificationFactory
.createBuilder()
.metaCategory(STOP_NOTIFICATION_CATEGORY)
.metaCategory(notificationCategory)
.metaType(HttpMimeType.JSON)
.message(Map.of("recording", desc, "target", targetId))
.message(Map.of("recording", linkedDesc, "target", targetId))
.build()
.send();
}
Expand Down Expand Up @@ -560,7 +571,10 @@ private void scheduleRecordingStopNotification(
.get()
.getReportURL(
connection, name));
this.notifyRecordingStopped(targetId, linkedDesc);
this.issueNotification(
targetId,
linkedDesc,
STOP_NOTIFICATION_CATEGORY);
}

return desc;
Expand Down
9 changes: 6 additions & 3 deletions src/test/java/io/cryostat/net/web/http/api/v1/TargetSnapshotPostHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ void shouldCreateSnapshot() throws Exception {
CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
Mockito.when(
recordingTargetHelper.verifySnapshot(
Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
Mockito.any(ConnectionDescriptor.class),
Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
.thenReturn(future2);
Mockito.when(future2.get()).thenReturn(true);

Expand Down Expand Up @@ -177,7 +178,8 @@ void shouldHandleSnapshotCreationExceptionDuringVerification() throws Exception
CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
Mockito.when(
recordingTargetHelper.verifySnapshot(
Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
Mockito.any(ConnectionDescriptor.class),
Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
.thenReturn(future2);
Mockito.when(future2.get())
.thenThrow(
Expand Down Expand Up @@ -215,7 +217,8 @@ void shouldHandleFailedSnapshotVerification() throws Exception {
CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
Mockito.when(
recordingTargetHelper.verifySnapshot(
Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
Mockito.any(ConnectionDescriptor.class),
Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
.thenReturn(future2);
Mockito.when(future2.get()).thenReturn(false);

Expand Down
9 changes: 6 additions & 3 deletions src/test/java/io/cryostat/net/web/http/api/v2/TargetSnapshotPostHandlerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ void shouldCreateSnapshot() throws Exception {
CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
Mockito.when(
recordingTargetHelper.verifySnapshot(
Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
Mockito.any(ConnectionDescriptor.class),
Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
.thenReturn(future2);
Mockito.when(future2.get()).thenReturn(true);

Expand Down Expand Up @@ -214,7 +215,8 @@ void shouldHandleSnapshotCreationExceptionDuringVerification() throws Exception
CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
Mockito.when(
recordingTargetHelper.verifySnapshot(
Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
Mockito.any(ConnectionDescriptor.class),
Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
.thenReturn(future2);
Mockito.when(future2.get())
.thenThrow(
Expand Down Expand Up @@ -254,7 +256,8 @@ void shouldHandleFailedSnapshotVerification() throws Exception {
CompletableFuture<Boolean> future2 = Mockito.mock(CompletableFuture.class);
Mockito.when(
recordingTargetHelper.verifySnapshot(
Mockito.any(ConnectionDescriptor.class), Mockito.eq("snapshot-1")))
Mockito.any(ConnectionDescriptor.class),
Mockito.any(HyperlinkedSerializableRecordingDescriptor.class)))
.thenReturn(future2);
Mockito.when(future2.get()).thenReturn(false);

Expand Down
31 changes: 28 additions & 3 deletions src/test/java/io/cryostat/recordings/RecordingTargetHelperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,8 +400,17 @@ void shouldVerifySnapshot() throws Exception {
Mockito.when(targetConnectionManager.markConnectionInUse(connectionDescriptor))
.thenReturn(true);

IRecordingDescriptor minimalDescriptor = createDescriptor(snapshotName);
HyperlinkedSerializableRecordingDescriptor snapshotDescriptor =
new HyperlinkedSerializableRecordingDescriptor(
minimalDescriptor,
"http://example.com/download",
"http://example.com/report");

boolean verified =
recordingTargetHelperSpy.verifySnapshot(connectionDescriptor, snapshotName).get();
recordingTargetHelperSpy
.verifySnapshot(connectionDescriptor, snapshotDescriptor)
.get();

Assertions.assertTrue(verified);
}
Expand All @@ -419,12 +428,19 @@ void shouldThrowSnapshotCreationExceptionWhenVerificationFails() throws Exceptio
Optional<InputStream> snapshotOptional = Optional.empty();
Mockito.when(future.get()).thenReturn(snapshotOptional);

IRecordingDescriptor minimalDescriptor = createDescriptor(snapshotName);
HyperlinkedSerializableRecordingDescriptor snapshotDescriptor =
new HyperlinkedSerializableRecordingDescriptor(
minimalDescriptor,
"http://example.com/download",
"http://example.com/report");

Assertions.assertThrows(
ExecutionException.class,
() -> {
try {
recordingTargetHelperSpy
.verifySnapshot(connectionDescriptor, snapshotName)
.verifySnapshot(connectionDescriptor, snapshotDescriptor)
.get();
} catch (ExecutionException ee) {
Assertions.assertTrue(ee.getCause() instanceof SnapshotCreationException);
Expand Down Expand Up @@ -475,8 +491,17 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
Mockito.when(recordingMetadataManager.getMetadata(Mockito.anyString(), Mockito.anyString()))
.thenReturn(new Metadata());

IRecordingDescriptor minimalDescriptor = createDescriptor(snapshotName);
HyperlinkedSerializableRecordingDescriptor snapshotDescriptor =
new HyperlinkedSerializableRecordingDescriptor(
minimalDescriptor,
"http://example.com/download",
"http://example.com/report");

boolean verified =
recordingTargetHelperSpy.verifySnapshot(connectionDescriptor, snapshotName).get();
recordingTargetHelperSpy
.verifySnapshot(connectionDescriptor, snapshotDescriptor)
.get();

Assertions.assertFalse(verified);
}
Expand Down