redis / lettuce

Advanced Java Redis client for thread-safe sync, async, and reactive usage. Supports Cluster, Sentinel, Pipelining, and codecs.
https://lettuce.io
MIT License
5.3k stars 947 forks source link

Extending recording/publishing infra #2859

Open rkeely opened 1 month ago

rkeely commented 1 month ago

Seems I messed up the previous PR a bit. Had to create a new one and carry the context here.

This PR is to fulfill this https://github.com/redis/lettuce/discussions/2809#discussioncomment-9055302 and address comments from https://github.com/redis/lettuce/pull/2819

Extending individual events requires duplicating a lot of the functionality within the individual events and it makes these mutable.

I was looking more for an approach where we extend the recording/publishing infrastructure. In particular:

  • Add Event getSource() to RecordableEvent and make RecordableEvent extends Event
  • Capture the original event in JfrRecordableEvent and rework NoOpEventRecorder to create an event instance
  • Long-lived events would be published to EventBus (e.g. EventRecorder.RecordableEvent event = …; later: eventBus.publish(event).
  • Adopt JfrEventRecorder.publish(…) to check whether the given event is EventRecorder.RecordableEvent. If so, then we call record(…) on the event instead of jdk.jfr.Event jfrEvent = createEvent(…);
  • Retrofit DefaultEventBus to unwrap EventRecorder.RecordableEvent before emitting the event

This is more about getting the direction correct. Will supply more testing if the infra change looks good.

Make sure that:

rkeely commented 4 weeks ago

@mp911de posted a new commits to address your last comment. Would you please take a look? Still its only changing the infra, and if it looks good, I am thinking to make another change that actually publish/record those RecordableEvent through EventBus.