docs: [FC-0074] add how-to add event bus support to an Open edX Event

[mariajgrimaldi]

Description

This PR adds a how-to guide for implementing an event with event bus support. It details the requirements for sending and receiving events through the event bus and how to ensure compatibility.

[openedx-webhooks]

Thanks for the pull request, @mariajgrimaldi!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/hooks-extension-framework. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[bmtcril]

Looking good, just a few little things

[bmtcril]

This might make more sense if you moved this into the Step 4 section.

[mariajgrimaldi]

The skipping part is slightly wrong. I meant "if you're using only supported data types, then you can skip implementing a custom serializer". But that sounds a bit redundant, considering L82. I dropped the line altogether: 901e495

I'm not sure why step 4 would be optional. Can you explain more about this?

[mariajgrimaldi]

@bmtcril: At first we thought that all events in the repo should have event bus support by default. So I was going to add support for these events: https://github.com/openedx/openedx-events/blob/main/openedx_events/tooling.py#L20-L32. However, I realized that we would also need to add support for dictionaries (typed and more complex) and/or a rewrite of the data classes, which requires a lot more effort than what we gain with the support since, as far as I understand, most of those events are locally scoped.

Do you think the question, "Does my Event Need Event Bus Support?" is relevant considering what I mentioned, and that should we study each event before compromising on event bus support?

FYI @sarina @felipemontoya

[felipemontoya]

I'm not sure I follow when you say locally scoped.

Some of the discussion thread events are very likely to be interesting by consumers of the event bus trying to make the discussion experience more reactive.

How much effort could it take to refactor those classes or to implement serialization capabilities for a list/dict with a limited capability of nesting. E.g: lists of primitives currently supported by avro.
Another option would be to say that we send most of the envelope of those discussion events and we keep the content of the discussion out of the serialization. The same we do with other objects such as a django user, we pass name, email and ID and we leave the consumer figure out the rest via API calls or such.

[mariajgrimaldi]

@felipemontoya: thanks for the reply!

I'm not sure I follow when you say locally scoped.

I meant for interest only within the service where it's sent. But as you said, you can argue that all events can be of interest to consumers. Do you think this section "Does my Event Need Event Bus Support?" is still relevant?

How much effort could it take to refactor those classes or to implement serialization capabilities for a list/dict with a limited capability of nesting. E.g: lists of primitives currently supported by avro.

I'm more concerned about doing it properly. We already have "support" for those limited nesting capabilities, i.e., using attrs classes for fixed dicts or JSON structs as strings when we don't know the content of the dicts beforehand. I already added a note here with the suggestion: https://github.com/openedx/openedx-events/pull/428/files#diff-67886caf4b3357c606ceb6d3ea25e3839b6056f13b22d48a146431adc0fa829dR120. So I don't think adding support for dicts is strictly necessary. I'm sorry I didn't mention this in my previous comment, so it read only that it was too much effort to add support.

But then we have dicts of lists or lists of data attrs which my guess is that is more difficult to serialize, or maybe we hadn't had a strong use case for it, and that's why it's not supported.

That's where I wondered, should an event always have support for the event bus or can we be flexible with that requirement? I totally understand why we wouldn't want the separation between events with/without event bus support, but that's where we currently stand.

Another option would be to say that we send most of the envelope of those discussion events and we keep the content of the discussion out of the serialization.

I was thinking we could create new event versions without the complex serializable sections and considering what I said about fixed/dynamic dicts, so we can rewrite the data and make it suitable for the event bus, but leave the previous versions to be sent within the service. However, that would require sending/maintaining two versions of the event.

[mariajgrimaldi]

As a result of this conversation, I started testing two approaches to give basic event bus support to data classes with dictionaries (str keys, only primitive types as values).

1. Send dicts as JSON structs (str): feat: add event bus support to forum events V2 #434
2. Add avro map support to dicts (based on this PR Cristhian opened a few months ago): feat: [FC-0074] add support for annotated python dicts as avro map type  #433

I was able to generate schemas for both approaches and avro tests seem to be passing, but I haven't tested them with an event bus implementation just yet. Let me know what you think!

[mariajgrimaldi]

I managed to send forum events through the event bus by using this implementation: #433

[felipemontoya]

Implementing support for the dict types by limiting to those that have annotations seems like a fair tradeoff for me. In my opinion this is the way to go to avoid having some events with support for the event bus and others without.

To give some context what I really hope we can avoid is to spend all our cards telling developers to build on top of the events framework and the event bus to later explain that we are limiting the events they can use over the bus to those that were supported for historical reasons. That would be such a miss in the dev-story for the framework.

[mariajgrimaldi]

@felipemontoya: supporting dicts doesn't necessarily add support for all events without event bus support, only for those with dict types in their payload, like forum events. But we still have events with more complex data structures like lists of attrs classes, which are not supported yet. That's why we should keep considering events that were not designed to be sent through the event bus, but keeping the support for the event bus as the default design choice. Would that be enough to move forward?

[bmtcril]

I'm a little late to the party here, but given the work you've been doing, are there remaining existing events that wouldn't serialize to the event bus once it merges? It would certainly be nice to make bus compatibility a requirement for new events going forward, since I constantly surprised at the different types of data people are trying to extract from the system.

If it's a lot of work, it seems like a thing we could detail in a ticket and come back to as time permits.

[mariajgrimaldi]

I added a note about maintainers enforcing event bus support for any new event to address these issues we're currently having from the beginning: https://docsopenedxorg--428.org.readthedocs.build/projects/openedx-events/en/428/how-tos/adding-event-bus-support-to-an-event.html#step-1-does-my-event-need-event-bus-support

As I mentioned above, I think there is more to do to add support to lists/dicts for more complex types, so I opened a follow up issue to keep working on it: #441. In the meantime, I'll play around a bit more to figure out what needs to be done.

[Ian2012]

Suggested change

	- The :term:`Event Payload` contains data types that are not supported by the event bus, and it is not possible to refactor the :term:`Event Payload` to use supported data types.
	- The :term:`Event Payload` contains data types that are not supported by the event bus (such as ...), and it is not possible to refactor the :term:`Event Payload` to use supported data types.

[mariajgrimaldi]

Thank you for the suggestion! Done 3b35a9d

[Ian2012]

Only an small nit. Is there the need to document how to send and consume an Open edX Event in the same runtime such as consuming events in the LMS from the event bus?

[mariajgrimaldi]

Only an small nit. Is there the need to document how to send and consume an Open edX Event in the same runtime such as consuming events in the LMS from the event bus?

@Ian2012: thanks for the review! Do you mean something like this? I'll be working on improving that document in the next couple of days as well.