Skip to content
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.

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

docs: [FC-0074] add ADR with event design suggestions #438

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
126 changes: 126 additions & 0 deletions docs/decisions/0016-event-design-practices.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
16. Event Design Practices
###########################

Status
------

Draft

Context
-------

It is important to follow standards to ensure that the events are consistent, maintainable, and reusable. The design of the events should be self-descriptive, self-contained, and provide enough information for consumers to understand the message. This ADR aims to provide a set of suggested practices for designing Open edX events.

Decision
--------

We have compiled a list of suggested practices taken from the following sources:

- `Event-Driven Microservices`_
- `Event-Driven article`_
- `Thin Events - The lean muscle of event-driven architecture`_

These are the practices that we recommend reviewing and following when designing an Open edX Event and contributing to the library. The goal is to implement events that are consistent with the architecture, reusable, and maintainable over time.

Event Purpose and Content
~~~~~~~~~~~~~~~~~~~~~~~~~

- An event should describe as accurately as possible what happened (what) and why it happened (why). It must contain enough information for consumers to understand the message. For instance, if an event is about a user enrollment, it should contain the user's data, the course data, and the enrollment status and the event should be named accordingly.
- Avoid immediately contacting the source service to retrieve additional information. Instead, consider adding the necessary information to the event payload by managing the granularity of the event. If the event requires additional information, consider adding a field to the event that contains the necessary information. This will reduce the number of dependencies between services and make the event more self-contained.
- Keep the event size small. Avoid adding unnecessary information to the event. If the information is not necessary for consumers to react to the event, consider removing it.
- Avoid adding flow-control information or business logic to events. Events should be solely a representation of what took place. If a field is necessary to control the behavior of the consumer, consider moving it to the consumer side. If adding additional data to the event is absolutely necessary document the reasoning behind it and carefully study the use case and implications.

Here is an example of an event that follows these practices which is emitted when the a user registers:

.. code-block:: python

# Location openedx_events/learning/signal.py
# .. event_type: org.openedx.learning.student.registration.completed.v1
# .. event_name: STUDENT_REGISTRATION_COMPLETED
# .. event_description: emitted when the user registration process in the LMS is completed.
# .. event_data: UserData
STUDENT_REGISTRATION_COMPLETED = OpenEdxPublicSignal(
event_type="org.openedx.learning.student.registration.completed.v1",
data={
"user": UserData,
}
)

Where:

- The event name indicates what happened: ``STUDENT_REGISTRATION_COMPLETED``.
- The event description explains why the event happened: ``emitted when the user registration process in the LMS is completed``.
- The event data contains data directly related to what happened ``UserData`` which should contain the necessary information to understand the event, like the username and email of the user.

Responsibility and Granularity
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

- Design events with a single responsibility in mind. Each event should represent a single action or fact that happened in the system. If an event contains multiple actions, consider splitting it into multiple events. For instance, if the course grade is updated to pass or fail, there should be two events: one for the pass action and another for the fail action.
- Manage the granularity of the event so it is not too coarse (generic with too much information) or too fine-grained (specific with too little information). When making a decision on the granularity of the event, start with the minimum required information for consumers to react to the event and add more information as needed with enough justification. If necessary, leverage API calls from the consumer side to retrieve additional information but always consider the trade-offs of adding dependencies with other services.
Copy link

Choose a reason for hiding this comment

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

I agree with the idea of avoid generic events, however, we should also avoid split events like course_passed or course_failed when we could use just course_completed with an status.

could we include a practical example on an appropiate level of granularity?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm interested in understanding why having a status is better. I think that in this case, it'd be easier to consume more granular events than leave the responsibility to the consumer to figure out whether the student passed or failed based on the status.

Copy link

Choose a reason for hiding this comment

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

I understand your perspective, and I think both approaches have valid use cases depending on the context, let me elaborate on why I suggested using course_completed with a status field and we can discuss further to find a balance

If the event represents a single conceptual action for example: completing a course, having one event like course_completed with a clear status passed, failed, etc. could simplify the producer's logic and reduce event proliferation and for consumers, interpreting the status field is relatively straightforward if it's well-documented and includes only a few well-defined values, however if different consumers need to handle passed and failed cases in significantly different ways, separate those events might reduce complexity for them but emitting separate events like course_passed and course_failed could also create challenges such as needing to ensure mutual exclusivty.

So in cases like this I think We could start with course_completed and a status field, ensuring it is well-documented and strictly validted and if in the future we observe a clear need for distinct event flows We could introduce the more granular events without breaking existing consumers.

Copy link
Member Author

@mariajgrimaldi mariajgrimaldi Dec 27, 2024

Choose a reason for hiding this comment

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

In the example we're using, I still think it'd be more straightforward to send smaller and more specific events. As I see it, course completion and grade passing would be two different critical facts happening in the system; therefore, they should be independent. This is more of a question of what consumers would want with a course completion event or a course passing status change.

In any case, these are the only suggestions that should be evaluated for each case. We currently have an event called COURSE_PASSING_STATUS_UPDATED which uses a similar flow control status field called is_passing. In that case, given that the status is directly related to the event, using a status field is acceptable. What do you think? Should we maybe use a rule of thumb indicating that an event can be split into more events, given that they could communicate more facts of the system?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@Alec4r I worry that your example is predicated on understanding the needs of the consumers. I don't think that's always possible, to predict how various consumers will need to consume events. I agree with @mariajgrimaldi that consuming an event that doesn't require me to also check the status field is conceptually simpler, even if it means I need to handle consuming a few discrete events.

- Ensure that the triggering logic is consistent and narrow. For instance, if an event is triggered when a user enrolls in a course, it should be trigger only when the user enrolls in a course in all ways possible to enroll in a course. If the event is triggered when a user enrolls in a course through the API, it should also be triggered when the user enrolls in a course through the UI.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Ensure that the triggering logic is consistent and narrow. For instance, if an event is triggered when a user enrolls in a course, it should be trigger only when the user enrolls in a course in all ways possible to enroll in a course. If the event is triggered when a user enrolls in a course through the API, it should also be triggered when the user enrolls in a course through the UI.
- Ensure that the triggering logic is consistent and narrow. For instance, if an event is triggered when a user enrolls in a course, it should be triggered when the user enrolls in a course in all ways possible to enroll in a course. If the event is triggered when a user enrolls in a course through the API, it should also be triggered when the user enrolls in a course through the UI.


For instance, consider the following events:

.. code-block:: python

# Location openedx_events/learning/signal.py
# .. event_type: org.openedx.learning.course.grade.passed.v1
# .. event_name: COURSE_GRADE_PASSED
# .. event_description: emitted when the user's course grade is updated to pass.
# .. event_data: CourseGradeData
COURSE_GRADE_PASSED = OpenEdxPublicSignal(
event_type="org.openedx.learning.course.grade.passed.v1",
data={
"grade": CourseGradeData,
}
)

# Location openedx_events/learning/signal.py
# .. event_type: org.openedx.learning.course.grade.failed.v1
# .. event_name: COURSE_GRADE_FAILED
# .. event_description: emitted when the user's course grade is updated to fail.
# .. event_data: CourseGradeData
COURSE_GRADE_FAILED = OpenEdxPublicSignal(
event_type="org.openedx.learning.course.grade.failed.v1",
data={
"grade": CourseGradeData,
}
)

Where:

- The event name indicates what happened: ``COURSE_GRADE_PASSED`` and ``COURSE_GRADE_FAILED``.
- The event description explains why the event happened: ``emitted when the user's course grade is updated to pass`` and ``emitted when the user's course grade is updated to fail``.
- The event data contains data directly related to what happened ``CourseGradeData`` which should contain the necessary information to understand the event, like the user, the course, the grade, and the date of the grade update.
- The granularity of the event is managed by having two events: one for the pass action and another for the fail action.

Each of these practices should be reviewed with each case, and the granularity of the event should be adjusted according to the use case and the information required by the consumers.

Event Structure and Clarity
~~~~~~~~~~~~~~~~~~~~~~~~~~~

- Use appropriate data types and formats for the event fields. Don't use generic data types like strings for all fields. Use specific data types like integers, floats, dates, or custom types when necessary.
- Avoid ambiguous data fields or fields with multiple meaning. For instance, if an event contains a field called ``status`` it should be clear what the status represents. If the status can have multiple meanings, consider splitting the event into multiple events or adding a new field to clarify the status.

For instance, consider the ``CourseEnrollmentData`` class:

- The ``mode`` field is a string that represents the course mode. It could be a string like "verified", "audit", "honor", etc.
- The ``is_active`` field is a boolean that represents whether the enrollment is active or not.
- The ``creation_date`` field is a datetime that represents the creation date of the enrollment.
- The ``created_by`` field is a ``UserData`` that represents the user who created the enrollment.
- The ``user`` field is a ``UserData`` that represents the user associated with the Course Enrollment.
- The ``course`` field is a ``CourseData`` that represents the course where the user is enrolled in.

Consumer-Centric Design
~~~~~~~~~~~~~~~~~~~~~~~

- When designing an event, consider the consumers that will be using it. What information do they need to react to the event? What data is necessary for them to process the event?
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- When designing an event, consider the consumers that will be using it. What information do they need to react to the event? What data is necessary for them to process the event?
- When designing an event, consider the consumers that will be using it. What information do they need to react to the event? What data is necessary for them to process the event?
- You can't always predict the needs of consumers in the future. Ensure your design is discrete, flexible, and well-documented, so new and novel use cases can be developed.

- Design events carefully from the start to minimize breaking changes for consumers, although it is not always possible to avoid breaking changes.
Copy link

Choose a reason for hiding this comment

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

will it be necessary to version events to handle event changes, or what is the plan for handling event changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Events are versioned by definition, see this ADR for more info. As for the evolution of the events schema, this other ADR describes what the behavior is supposed to be: https://github.com/openedx/openedx-events/blob/main/docs/decisions/0006-event-schema-serialization-and-evolution.rst#decision, although according to this comment the ADR might be outdated -- I'll be working on updating it. The reality is that we haven't needed to change an event definition in any way that's breaking.

Copy link
Member Author

Choose a reason for hiding this comment

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

@robrap: Can you help us figure out what needs to change from the ADR-0006? I could do it, but I need more context to do it effectively.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @mariajgrimaldi. I was off for the last two weeks, so just getting back to things.

It seems like you are pretty up to date. I'll summarize what I think you are already saying.

  1. As noted in the comment you linked to, the existence of optional fields probably changes the 0006 recommendation. The first sentence of the deferred alternatives in the ADR reads: "We could add the ability to add optional fields to an event...". And we did add this ability.
  2. Most importantly, no events have actually gone through any process of evolution, so all of this is just best-guesses about what we'd like to do and how we think it would work. It was deemed too much effort to proceed any further without an actual need.

Does this help? Let me know if you have any specific questions.


Some of these practices might not be applicable to all events, but they are a good starting point to ensure that the events are consistent and maintainable over time. So, design the event so it is small, well-defined and only contain relevant information.

In addition to these practices, review the Architectural Decision Records (ADRs) related to events to understand the naming, versioning, payload, and other practices that are specific to Open edX events.

.. _Event-Driven Microservices: https://www.oreilly.com/library/view/building-event-driven-microservices/9781492057888/
.. _Event-Driven article: https://martinfowler.com/articles/201701-event-driven.html
.. _Thin Events - The lean muscle of event-driven architecture: https://www.thoughtworks.com/insights/blog/architecture/thin-events-the-lean-muscle-of-event-driven-architecture
1 change: 1 addition & 0 deletions docs/decisions/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ Architectural Decision Records (ADRs)
0013-special-exam-submission-and-review-events
0014-new-event-bus-producer-config
0015-outbox-pattern-and-production-modes
0016-event-design-practices
Loading