Skip to content

Add tableProperties to TableAuditEvent #601

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

Open
james5418 wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −1
Open

Add tableProperties to TableAuditEvent #601

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
8 changes: 7 additions & 1 deletion services/tables/src/main/java/com/linkedin/openhouse/tables/audit/TableAuditAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -371,12 +371,18 @@ protected ApiResponse<GetTableResponseBody> auditPutIcebergSnapshots(
} else {
operationType = OperationType.COMMIT;
}
CreateUpdateTableRequestBody createUpdateTableRequestBody =
icebergSnapshotRequestBody.getCreateUpdateTableRequestBody();
TableAuditEvent.TableAuditEventBuilder eventBuilder =
TableAuditEvent.builder()
.eventTimestamp(Instant.now())
.databaseName(databaseId)
.tableName(tableId)
.operationType(operationType);
.operationType(operationType)
.tableProperties(
createUpdateTableRequestBody == null
Copy link
Copy Markdown
Collaborator

@cbb330 cbb330 May 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tableProperties is read from createUpdateTableRequestBody (the client's request payload), not from the actual committed Iceberg table state. If OpenHouse adds or mutates properties server-side during commit (e.g. openhouse.tableVersion, openhouse.lastModifiedTime), those won't appear in the audit event.

The fix can be to get it from the response body. this response from claude looks like good next steps.

The fix is straightforward given what the aspect already has. result (the ApiResponse) is available after point.proceed(), and
result.getResponseBody() already has getTableLocation() called on it — getTableProperties() is right there too.
The change in TableAuditAspect.java: remove tableProperties from the pre-proceed() builder, delay .build() until after proceed() so you can read from the
response:

  // Remove the createUpdateTableRequestBody variable entirely.
  // Remove .tableProperties(...) from the builder chain.
  // Change the try block to:
  try {
    result = (ApiResponse<GetTableResponseBody>) point.proceed();
    TableAuditEvent event = eventBuilder
        .tableProperties(result.getResponseBody().getTableProperties())
        .build();
    buildAndSendEvent(event, OperationStatus.SUCCESS, result.getResponseBody().getTableLocation());
  } catch (Throwable t) {
    buildAndSendEvent(eventBuilder.build(), OperationStatus.FAILED, null);
    throw t;
  }

On the failure path eventBuilder.build() has no tableProperties — which is correct, there's no committed state to read from.
One thing to verify before doing this: confirm GetTableResponseBody has getTableProperties(). It almost certainly does since the response describes full
table state, but worth a quick grep in the codebase.

? null
: createUpdateTableRequestBody.getTableProperties());
extractSnapshotInfo(icebergSnapshotRequestBody, eventBuilder);
TableAuditEvent event = eventBuilder.build();
try {
Expand Down
3 changes: 3 additions & 0 deletions services/tables/src/main/java/com/linkedin/openhouse/tables/audit/model/TableAuditEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.linkedin.openhouse.common.audit.model.BaseAuditEvent;
import java.time.Instant;
import java.util.Map;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.EqualsAndHashCode;
Expand Down Expand Up @@ -40,4 +41,6 @@ public class TableAuditEvent extends BaseAuditEvent {
private Long currentSnapshotId;

private Long currentSnapshotTimestampMs;

private Map<String, String> tableProperties;
}
10 changes: 10 additions & 0 deletions .../tables/src/main/java/com/linkedin/openhouse/tables/config/InternalCatalogProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package com.linkedin.openhouse.tables.config;

import java.time.Duration;
import java.util.Collections;
import java.util.List;
import lombok.Getter;
import lombok.Setter;
import org.springframework.boot.context.properties.ConfigurationProperties;
Expand All @@ -13,11 +15,19 @@ public class InternalCatalogProperties {

private MetadataCache metadataCache = new MetadataCache();

private Audit audit = new Audit();

@Getter
@Setter
public static class MetadataCache {
private Boolean enabled;
private Duration ttl;
private DataSize maxWeight;
}

@Getter
@Setter
public static class Audit {
private List<String> tablePropertiesAllowlist = Collections.emptyList();
}
}
31 changes: 31 additions & 0 deletions ...st/java/com/linkedin/openhouse/tables/mock/audit/IcebergSnapshotsApiHandlerAuditTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,37 @@ public void testPutIcebergSnapshotsMainPointsToOlderSnapshot() throws Exception
assertEquals(1000L, actualEvent.getCurrentSnapshotTimestampMs().longValue());
}

@Test
public void testPutIcebergSnapshotsCarriesTableProperties() throws Exception {
Map<String, String> properties = new HashMap<>();
properties.put("openhouse.watermark", "100");
properties.put("openhouse.tableType", "PRIMARY_TABLE");
properties.put("user.custom.key", "v");
IcebergSnapshotsRequestBody base = RequestConstants.TEST_ICEBERG_SNAPSHOTS_REQUEST_BODY;
IcebergSnapshotsRequestBody requestBody =
IcebergSnapshotsRequestBody.builder()
.baseTableVersion(base.getBaseTableVersion())
.jsonSnapshots(base.getJsonSnapshots())
.snapshotRefs(base.getSnapshotRefs())
.createUpdateTableRequestBody(
base.getCreateUpdateTableRequestBody()
.toBuilder()
.tableProperties(properties)
.build())
.build();
mvc.perform(
MockMvcRequestBuilders.put(
String.format(
CURRENT_MAJOR_VERSION_PREFIX
+ "/databases/d200/tables/tb1/iceberg/v2/snapshots"))
.accept(MediaType.APPLICATION_JSON)
.contentType(MediaType.APPLICATION_JSON)
.content(requestBody.toJson()));
Mockito.verify(tableAuditHandler, atLeastOnce()).audit(argCaptor.capture());
TableAuditEvent actualEvent = argCaptor.getValue();
assertEquals(properties, actualEvent.getTableProperties());
}

@Test
public void testCTASCommitPhase() throws Exception {
mvc.perform(
Expand Down