feat: add structured metrics for deleteIfUnchanged outcomes in GlueSync

hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSync.java

@@ -186,15 +186,9 @@ public void onDropTable(DropTableEvent event) throws MetaException {
     }
     Table table = event.getTable();
     try {
-      glueTableService.deleteIfUnchanged(table);
+      GlueTableService.DeleteOutcome outcome = glueTableService.deleteIfUnchanged(table);
       metricService.incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS);
-      metricService.recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "deleted");
-    } catch (EntityNotFoundException e) {
-      log.info(table + " table doesn't exist in glue catalog");
-      metricService.recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "not_found");
-      if (throwExceptions) {
-        throw wrap(e);
-      }
+      metricService.recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, outcome.metricOutcome());
     } catch (Exception e) {
       log.error("Failed drop table {}.{} in glue", table.getDbName(), table.getTableName(), e);
       metricService.incrementCounter(MetricConstants.LISTENER_TABLE_FAILURE);
@@ -293,10 +287,10 @@ private void doRenameOperation(Table oldTable, Table newTable) {
     long startTime = System.currentTimeMillis();
     glueTableService.create(newTable);
     gluePartitionService.copyPartitions(newTable, gluePartitionService.getPartitions(oldTable));
-    glueTableService.deleteIfUnchanged(oldTable);
-    metricService.incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS);
-    metricService.recordEvent(MetricConstants.ALTER_TABLE, MetricConstants.RESULT_SUCCESS, "renamed");
+    GlueTableService.DeleteOutcome outcome = glueTableService.deleteIfUnchanged(oldTable);
     long duration = System.currentTimeMillis() - startTime;
+    metricService.incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS);
+    metricService.recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, outcome.metricOutcome());
     metricService.recordDuration(MetricConstants.LISTENER_TABLE_RENAME_DURATION, duration);
     log.info("{} glue table rename to {} finished in {}ms", oldTable.getTableName(), newTable.getTableName(), duration);
   }

hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/metrics/MetricConstants.java

@@ -46,6 +46,7 @@ public class MetricConstants {
   public static final String CREATE_TABLE    = "create_table";
   public static final String DROP_TABLE      = "drop_table";
   public static final String ALTER_TABLE     = "alter_table";
+  public static final String RENAME_TABLE    = "rename_table";
   public static final String ADD_PARTITION   = "add_partition";
   public static final String DROP_PARTITION  = "drop_partition";
   public static final String ALTER_PARTITION = "alter_partition";

hive-event-listeners/apiary-gluesync-listener/src/main/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableService.java

@@ -34,6 +34,35 @@
 import com.amazonaws.services.glue.model.ValidationException;
 
 public class GlueTableService {
+
+  /**
+   * Outcome of a {@link #deleteIfUnchanged} call, used by callers to record the appropriate metric.
+   * <ul>
+   *   <li>{@code DELETED} — the Glue table was deleted.</li>
+   *   <li>{@code SKIPPED} — the delete was suppressed because identity params diverged, indicating
+   *       the Glue slot was overwritten by a concurrent operation.</li>
+   *   <li>{@code NOT_FOUND} — the table did not exist in Glue when checked; nothing to delete.</li>
+   *   <li>{@code DELETED_CONCURRENTLY} — the table existed at guard-check time but was deleted by a
+   *       concurrent operation before the delete call completed (TOCTOU race).</li>
+   * </ul>
+   */
+  public enum DeleteOutcome {
+    DELETED("deleted"),
+    SKIPPED("delete_skipped"),
+    NOT_FOUND("not_found"),
+    DELETED_CONCURRENTLY("concurrent_delete");
+
+    private final String metricOutcome;
+
+    DeleteOutcome(String metricOutcome) {
+      this.metricOutcome = metricOutcome;
+    }
+
+    public String metricOutcome() {
+      return metricOutcome;
+    }
+  }
+
   private static final Logger log = LoggerFactory.getLogger(GlueTableService.class);
   // HMS parameters checked before a delete to detect if the table slot was overwritten by a concurrent operation
   private static final String[] IDENTITY_PARAMS = { "transient_lastDdlTime", "metadata_location" };
@@ -112,7 +141,7 @@ public void delete(Table table) {
    * delete will be skipped, leaving an orphan in Glue. An orphan is intentionally preferred over
    * incorrectly deleting a table that has since been overwritten by a rename.
    */
-  public void deleteIfUnchanged(Table table) {
+  public DeleteOutcome deleteIfUnchanged(Table table) {
     Map<String, String> hmsParams = table.getParameters() != null ? table.getParameters() : Collections.emptyMap();
     com.amazonaws.services.glue.model.Table glueTable;
     try {
@@ -123,17 +152,24 @@ public void deleteIfUnchanged(Table table) {
           .getTable();
     } catch (EntityNotFoundException e) {
       log.info("{}.{} table not found in glue catalog, nothing to delete", table.getDbName(), table.getTableName());
-      return;
+      return DeleteOutcome.NOT_FOUND;
     }
     Map<String, String> glueParams = glueTable.getParameters() != null ? glueTable.getParameters() : Collections.emptyMap();
 
     for (String key : IDENTITY_PARAMS) {
       if (paramChanged(key, hmsParams, glueParams, table)) {
-        return;
+        return DeleteOutcome.SKIPPED;
       }
     }
 
-    delete(table);
+    try {
+      delete(table);
+    } catch (EntityNotFoundException e) {
+      log.info("{}.{} table deleted from glue catalog between guard check and delete",
+          table.getDbName(), table.getTableName());
+      return DeleteOutcome.DELETED_CONCURRENTLY;
+    }
+    return DeleteOutcome.DELETED;
   }
 
   private boolean paramChanged(String key, Map<String, String> hmsParams, Map<String, String> glueParams, Table table) {

hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/ApiaryGlueSyncTest.java

@@ -482,7 +482,7 @@ public void onAlterHiveTable_RenameTable() throws MetaException {
     BatchCreatePartitionRequest batchCreatePartitionRequest = batchCreatePartitionRequestCaptor.getValue();
     verify(glueClient).deleteTable(deleteTableRequestCaptor.capture());
     verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS);
-    verify(metricService).recordEvent(MetricConstants.ALTER_TABLE, MetricConstants.RESULT_SUCCESS, "renamed");
+    verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, "deleted");
     verify(metricService).recordDuration(eq(MetricConstants.LISTENER_TABLE_RENAME_DURATION), anyLong());
     DeleteTableRequest deleteTableRequest = deleteTableRequestCaptor.getValue();
 
@@ -721,16 +721,63 @@ public void onDropTableThatDoesntExistInGlue() throws MetaException {
     DropTableEvent event = mock(DropTableEvent.class);
     when(event.getStatus()).thenReturn(true);
     when(event.getTable()).thenReturn(simpleHiveTable(simpleSchema(), simplePartitioning()));
-    when(glueClient.getTable(any())).thenReturn(new GetTableResult().withTable(new com.amazonaws.services.glue.model.Table()));
-    when(glueClient.deleteTable(any())).thenThrow(new EntityNotFoundException(""));
+    when(glueClient.getTable(any())).thenThrow(new EntityNotFoundException(""));
 
     glueSync.onDropTable(event);
 
-    verify(glueClient).deleteTable(any());
+    verify(glueClient, never()).deleteTable(any());
+    verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS);
     verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "not_found");
     verifyNoMoreInteractions(metricService);
   }
 
+  @Test
+  public void onDropTable_skipsDeleteWhenGuardFires() throws MetaException {
+    DropTableEvent event = mock(DropTableEvent.class);
+    when(event.getStatus()).thenReturn(true);
+    Table hmsTable = simpleHiveTable(simpleSchema(), simplePartitioning());
+    hmsTable.putToParameters("transient_lastDdlTime", "100");
+    when(event.getTable()).thenReturn(hmsTable);
+    when(glueClient.getTable(any())).thenReturn(
+        new GetTableResult().withTable(new com.amazonaws.services.glue.model.Table()
+            .withParameters(ImmutableMap.of("transient_lastDdlTime", "999"))));
+
+    glueSync.onDropTable(event);
+
+    verify(glueClient, never()).deleteTable(any());
+    verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS);
+    verify(metricService).recordEvent(MetricConstants.DROP_TABLE, MetricConstants.RESULT_SUCCESS, "delete_skipped");
+    verifyNoMoreInteractions(metricService);
+  }
+
+  @Test
+  public void onAlterHiveTable_RenameTable_skipsOldTableDeleteWhenGuardFires() throws MetaException {
+    AlterTableEvent event = mock(AlterTableEvent.class);
+    when(event.getStatus()).thenReturn(true);
+
+    Table oldTable = simpleHiveTable(simpleSchema(), simplePartitioning());
+    oldTable.setTableName("table2");
+    oldTable.putToParameters("transient_lastDdlTime", "100");
+    when(event.getOldTable()).thenReturn(oldTable);
+
+    Table newTable = simpleHiveTable(simpleSchema(), simplePartitioning());
+    newTable.setTableName("table2_new");
+    when(event.getNewTable()).thenReturn(newTable);
+
+    when(glueClient.getPartitions(any())).thenReturn(new GetPartitionsResult().withPartitions(Collections.emptyList()));
+    // Glue has a different DDL time — guard fires, old table slot not deleted
+    when(glueClient.getTable(any())).thenReturn(
+        new GetTableResult().withTable(new com.amazonaws.services.glue.model.Table()
+            .withParameters(ImmutableMap.of("transient_lastDdlTime", "999"))));
+
+    glueSync.onAlterTable(event);
+
+    verify(glueClient, never()).deleteTable(any());
+    verify(metricService).incrementCounter(MetricConstants.LISTENER_TABLE_SUCCESS);
+    verify(metricService).recordEvent(MetricConstants.RENAME_TABLE, MetricConstants.RESULT_SUCCESS, "delete_skipped");
+    verify(metricService).recordDuration(eq(MetricConstants.LISTENER_TABLE_RENAME_DURATION), anyLong());
+  }
+
   @Test
   public void onAlterHiveTable_RenameOperationFailureIsMetered() throws MetaException {
     AlterTableEvent event = mock(AlterTableEvent.class);

hive-event-listeners/apiary-gluesync-listener/src/test/java/com/expediagroup/apiary/extensions/gluesync/listener/service/GlueTableServiceTest.java

@@ -15,6 +15,8 @@
  */
 package com.expediagroup.apiary.extensions.gluesync.listener.service;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -62,17 +64,19 @@ public void setUp() {
   public void deleteIfUnchanged_deletesWhenAllPropertiesMatch() {
     when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, null));
 
-    service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.DELETED));
     verify(glueClient).deleteTable(any(DeleteTableRequest.class));
   }
 
   @Test
   public void deleteIfUnchanged_skipsWhenLastDdlTimeChanged() {
     when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, "1751385999", null));
 
-    service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED));
     verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class));
   }
 
@@ -82,8 +86,9 @@ public void deleteIfUnchanged_skipsWhenHiveTableReplacedByIceberg() {
     when(glueClient.getTable(any(GetTableRequest.class)))
         .thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json"));
 
-    service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED));
     verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class));
   }
 
@@ -92,37 +97,53 @@ public void deleteIfUnchanged_skipsWhenMetadataLocationDiverges() {
     when(glueClient.getTable(any(GetTableRequest.class)))
         .thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v2.metadata.json"));
 
-    service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json"));
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(
+        hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json"));
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED));
     verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class));
   }
 
   @Test
   public void deleteIfUnchanged_skipsWhenHmsHasMetadataLocationButGlueDoesNot() {
     when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, null));
 
-    service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json"));
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(
+        hmsTable(LOCATION, LAST_DDL_TIME, "s3://bucket/test_table/metadata/v1.metadata.json"));
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED));
     verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class));
   }
 
   @Test
-  public void deleteIfUnchanged_skipsWhenTableNotFoundInGlue() {
+  public void deleteIfUnchanged_notFoundWhenTableAbsentDuringGet() {
     when(glueClient.getTable(any(GetTableRequest.class))).thenThrow(new EntityNotFoundException("not found"));
 
-    service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.NOT_FOUND));
     verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class));
   }
 
+  @Test
+  public void deleteIfUnchanged_deletedConcurrentlyWhenTableDeletedBetweenGuardAndDelete() {
+    when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResult(LOCATION, LAST_DDL_TIME, null));
+    when(glueClient.deleteTable(any(DeleteTableRequest.class))).thenThrow(new EntityNotFoundException("not found"));
+
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
+
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.DELETED_CONCURRENTLY));
+  }
+
   @Test
   public void deleteIfUnchanged_deletesWhenBothParamMapsAreNull() {
     when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResultNullParams());
 
     Table table = hmsTable(LOCATION, null, null);
     table.setParameters(null);
-    service.deleteIfUnchanged(table);
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(table);
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.DELETED));
     verify(glueClient).deleteTable(any(DeleteTableRequest.class));
   }
 
@@ -132,17 +153,19 @@ public void deleteIfUnchanged_skipsWhenHmsParamsNullButGlueHasParams() {
 
     Table table = hmsTable(LOCATION, null, null);
     table.setParameters(null);
-    service.deleteIfUnchanged(table);
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(table);
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED));
     verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class));
   }
 
   @Test
   public void deleteIfUnchanged_skipsWhenGlueParamsNullButHmsHasParams() {
     when(glueClient.getTable(any(GetTableRequest.class))).thenReturn(glueTableResultNullParams());
 
-    service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
+    GlueTableService.DeleteOutcome outcome = service.deleteIfUnchanged(hmsTable(LOCATION, LAST_DDL_TIME, null));
 
+    assertThat(outcome, is(GlueTableService.DeleteOutcome.SKIPPED));
     verify(glueClient, never()).deleteTable(any(DeleteTableRequest.class));
   }