Event type IDs + event metadata incl. OTel context #2998

adutra · 2025-11-06T17:48:04Z

This PR implements the action items from the following discussion threads:

Summary of changes:

Introduced a PolarisEventType enum holding the 150+ event types.
Introduced a PolarisEventMetadata interface as suggested by @adnanhemani, exposing: timestamp, realm ID, principal, request ID, and OTel context.
Introduced a PolarisEventMetadataFactory to centralize the logic for gathering the various elements of an event metadata.
Modified PolarisEvent to expose 3 new methods:
- UUID id()
- PolarisEventType type()
- PolarisEventMetadata metadata()
Persistence of OTel context is done in additional_properties as suggested by @flyrain.
Added InMemoryBufferEventListenerIntegrationTest to verify that all contextual data is properly persisted.

Checklist

🛡️ Don't disclose security issues! (contact security@apache.org)
🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
🧪 Added/updated tests with good coverage, or manually tested (and explained how)
💡 Added comments for complex logic
🧾 Updated CHANGELOG.md (if needed)
📚 Updated documentation in site/content/in-dev/unreleased (if needed)

adutra · 2025-11-06T17:48:56Z

\cc @flyrain @adnanhemani @dimas-b

adnanhemani

Overall, looks great. Thank you for this, @adutra! One question that I think might help make this a tighter change, but overall no concerns!

adnanhemani · 2025-11-07T22:29:13Z

...ice/src/main/java/org/apache/polaris/service/admin/PolarisCatalogsEventServiceDelegator.java

    polarisEventListener.onBeforeCreateCatalog(
-        new CatalogsServiceEvents.BeforeCreateCatalogEvent(request.getCatalog().getName()));
+        new CatalogsServiceEvents.BeforeCreateCatalogEvent(
+            PolarisEvent.createEventId(),


Wanted to see your thoughts on this: can we collapse Event ID into the EventMetadata as well? I think it makes sense as part of the "metadata" of the event and may help us save many lines of code uniformly across all events.

I thought about that too, but after some hesitation, I think an ID is a bit more than "metadata". The fact that this specific field must be unique makes it imho a bit special. Wdyt?

That said I don't have strong opinions on this, and I'd be OK to move it to metadata if there is a consensus to do so.

I understand your point, it is definitely a bit of a "grey area." I think the amount of cleanliness that we gain from moving it into EventMetadata outweighs the ambiguities here, though. I'm also willing to sway my opinion with the will of the community, but I would vote for merging it into EventMetadata if others feel similarly :)

The more I think about this the more I think we shouldn't move the id to the metadata. Here is what the PolarisEventMetadata would look like:

@PolarisImmutable public interface PolarisEventMetadata { UUID id(); // other methods omitted for brevity }

But that creates an ambiguity: is id() the identifier of the metadata object, or of the event object?

I could rename it to eventId() of course, but it still doesn't feel right to me. An identifier should be attached to the object it identifies.

I'd say it would be named eventId() in this model. IMO, the rest of the fields in PolarisEventMetadata are also vital parts of the event's identity - but I understand the argument philosophically. It's ultimately your call, but I still don't see the harm of doing this versus the benefits are the conciseness of the code.

Fair enough, I moved the id to PolarisEventMetadata. Please take another look at the latest commit 881a477

adnanhemani · 2025-11-13T02:13:44Z

@adutra - I see you force-pushed the last commit so it's rewritten the commit history. I'm assuming it was for rebasing this PR onto main? Was there any other major change? Since it's a larger change, I fear I may have missed something.

adutra · 2025-11-13T11:31:29Z

@adutra - I see you force-pushed the last commit so it's rewritten the commit history. I'm assuming it was for rebasing this PR onto main? Was there any other major change? Since it's a larger change, I fear I may have missed something.

I rebased because of conflicts - no functional change.

dimas-b · 2025-11-17T18:02:15Z

@adutra : there are more conflicts 🤷

adutra · 2025-11-18T23:03:40Z

@adutra : there are more conflicts 🤷

re-rebased!

@adnanhemani

This PR implements the action items from the following discussion threads: - https://lists.apache.org/thread/yx7pkgczl6k7bt4k4yzqrrq9gn7gqk2p - https://lists.apache.org/thread/rl5cpcft16sn5n00mfkmx9ldn3gsqtfy - https://lists.apache.org/thread/5dpyo0nn2jbnjtkgv0rm1dz8mpt132j9 Summary of changes: - Introduced a `PolarisEventType` enum holding the 150+ event types. - Introduced a `PolarisEventMetadata` interface as suggested by @adnanhemani, exposing: timestamp, realm ID, principal, request ID, and OTel context. - Introduced a `PolarisEventMetadataFactory` to centralize the logic for gathering the various elements of an event metadata. - Modified `PolarisEvent` to expose 3 new methods: - `UUID id()` - `PolarisEventType type()` - `PolarisEventMetadata metadata()` - Persistence of OTel context is done in `additional_properties` as suggested by @flyrain. - Added `InMemoryBufferEventListenerIntegrationTest` to verify that all contextual data is properly persisted.

adutra · 2025-11-25T15:11:36Z

FYI: re-rebased because of more conflicts.

adnanhemani

LGTM! Thank you for this @adutra!!

dimas-b · 2025-11-26T22:39:48Z

runtime/service/src/main/java/org/apache/polaris/service/events/PolarisEventMetadata.java

+
+  /** The unique ID of the event. */
+  default UUID eventId() {
+    return UUID.randomUUID();


Same concern as for request IDs, if it is used for each event (likely multiple per request), it might be too slow / too much affected by the system source of randomness... but we can address this later if other people think it's a concern.

dimas-b · 2025-11-26T22:41:23Z

...ime/service/src/main/java/org/apache/polaris/service/events/PolarisEventMetadataFactory.java

+   * "UNKNOWN_REALM_ID".
+   */
+  private String getRealmId() {
+    return realmContext.isResolvable()


Why are we not sure that realm context is always present?

In some tests, events are generated on the "client side", in which case, there is no realm context. (In production code there is always a ream context though.)

dimas-b · 2025-11-26T22:46:45Z

runtime/service/build.gradle.kts

  }

  testImplementation(project(":polaris-api-management-model"))
+  testImplementation(project(":polaris-relational-jdbc"))


I might have missing this... How does JDBC get into tests now ? 😅

To test the persistence-in-memory-buffer event listener with H2: see InMemoryBufferEventListenerIntegrationTest.

github-project-automation bot added this to Basic Kanban Board Nov 6, 2025

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board Nov 6, 2025

adnanhemani reviewed Nov 7, 2025

View reviewed changes

adutra force-pushed the events-api-refactoring branch from 1fd437e to c84c18a Compare November 12, 2025 14:44

adutra force-pushed the events-api-refactoring branch from c84c18a to 18f92fc Compare November 18, 2025 23:03

adutra added 2 commits November 25, 2025 12:09

move event.id()

881a477

adutra force-pushed the events-api-refactoring branch from 4c64677 to 881a477 Compare November 25, 2025 15:10

adnanhemani approved these changes Nov 26, 2025

View reviewed changes

dimas-b approved these changes Nov 26, 2025

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Nov 26, 2025

adutra merged commit 64b13a9 into apache:main Nov 27, 2025
15 checks passed

adutra deleted the events-api-refactoring branch November 27, 2025 15:16

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Nov 27, 2025

Event type IDs + event metadata incl. OTel context #2998

Event type IDs + event metadata incl. OTel context #2998

Uh oh!

Conversation

adutra commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

adutra commented Nov 6, 2025

Uh oh!

adnanhemani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adutra Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adnanhemani commented Nov 13, 2025

Uh oh!

adutra commented Nov 13, 2025

Uh oh!

dimas-b commented Nov 17, 2025

Uh oh!

adutra commented Nov 18, 2025

Uh oh!

adutra commented Nov 25, 2025

Uh oh!

adnanhemani left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adutra commented Nov 6, 2025 •

edited

Loading

adutra Nov 13, 2025 •

edited

Loading