Skip to content

feat: migrate to mitol-django-observability plugin#3001

Merged
feoh merged 70 commits intomainfrom
feat/mitol-django-observability
Mar 20, 2026
Merged

feat: migrate to mitol-django-observability plugin#3001
feoh merged 70 commits intomainfrom
feat/mitol-django-observability

Conversation

@blarghmatey
Copy link
Copy Markdown
Member

Summary

Migrate to mitol-django-observability — the new shared plugin that consolidates OpenTelemetry tracing and structured logging configuration.

Changes

  • Remove main/telemetry.pyconfigure_opentelemetry() is now called automatically by the plugin
  • Remove LOGGING dict from settings.py — provided by the plugin via mitol.observability.settings.logging
  • Add mitol.observability.apps.ObservabilityConfig to INSTALLED_APPS
  • Update pyproject.toml: add mitol-django-observability dep, remove direct opentelemetry-api/sdk/exporter deps

Before / After

Before: ~120 lines of boilerplate OTel + logging setup across telemetry.py, apps.py, and settings.py
After: One entry in INSTALLED_APPS + one import

Depends on

Requires mitol-django-observability to be published to PyPI (see ol-django#407).

Replace bespoke OpenTelemetry + logging boilerplate with the new
mitol-django-observability shared plugin.

Changes:
- Remove main/telemetry.py (configure_opentelemetry moved to plugin)
- Remove LOGGING dict from settings.py (provided by plugin)
- Add 'from mitol.observability.settings.logging import LOGGING'
- Add 'mitol.observability.apps.ObservabilityConfig' to INSTALLED_APPS
- Update pyproject.toml: add mitol-django-observability dep,
  remove direct opentelemetry-api/sdk/exporter deps

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 2, 2026

OpenAPI Changes

Show/hide No detectable change.

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

Copy link
Copy Markdown
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

I tried modifying pyproject.toml to point to the ol-django commit for that PR, to see if that would fix the failing tests:

"mitol-django-observability @ git+ssh://git@github.com/mitodl/ol-django.git@a0493006d1cdf8553ff02556e9bf8af346743f55#subdirectory=src/observability",

But ended up getting a bunch of errors on uv sync, looks like additional libraries need to be updated:

× No solution found when resolving dependencies:
╰─▶ Because mitol-django-common==2025.12.23.2 depends on django-redis>=6.0,<7.dev0 and only mitol-django-common==2025.12.23.2 is available, we can conclude
that all versions of mitol-django-common depend on django-redis>=6.0,<7.dev0.
And because mitol-django-observability==2026.1.0 depends on mitol-django-common and only mitol-django-observability==2026.1.0 is available, we can
conclude that all versions of mitol-django-observability depend on django-redis>=6.0,<7.dev0.
And because your project depends on django-redis>=5.2.0,<6 and mitol-django-observability, we can conclude that your project's requirements are
unsatisfiable.

Copilot AI review requested due to automatic review settings March 20, 2026 15:07
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Migrates the Django project’s tracing/logging setup to the shared mitol-django-observability plugin, removing local OpenTelemetry/bootstrap code and deferring configuration to the plugin.

Changes:

  • Add mitol-django-observability as a dependency and update the lockfile accordingly.
  • Remove the project’s bespoke OpenTelemetry initializer (main/telemetry.py) and its invocation from main/apps.py.
  • Switch Django logging configuration to use mitol.observability.settings.logging.LOGGING and register the plugin app config.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
uv.lock Adds mitol-django-observability (and its transitive deps like structlog) and removes direct OTel deps.
pyproject.toml Adds mitol-django-observability and removes direct opentelemetry-* API/SDK/exporter deps.
main/telemetry.py Removes local OpenTelemetry configuration module.
main/settings.py Adds observability app to INSTALLED_APPS and imports plugin-provided LOGGING.
main/apps.py Removes explicit OpenTelemetry initialization in app ready().

Comment thread pyproject.toml Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

uv sync might need to run one more time?

Noticed this difference in uv.lock when I ran it locally:

    { name = "mitol-django-observability", specifier = ">=2026.1.0" },

vs

    { name = "mitol-django-observability", specifier = ">=2026.1.0,<2027" },

gumaerc and others added 19 commits March 20, 2026 14:02
)

* handle additional sorting / filtering conditions for my learning, such as only having expired course run enrollments, only program enrollments, etc.

* clarify test comment

* remove redundant variable assignment

* consolidate resources passed into enrollmentexpandcollapse as dashboardresources
* fix: make direct call to cdn purge


---------

Co-authored-by: Ahtesham Quraish <ahtesham.quraish@192.168.10.4>
* switch to new format for resource id

* fixing tests

* prefer dictionary resource id - fall back to learning_resource_id
* bump mitxonline client

* fix tests and ts errors

* use new cart apis

* bump client

* bump client version

* update summary price display

* show financial aid pricing in course summary

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* update enrollment buttons for paid enrollment and financial aid

Show "Enroll Now—$X" for paid-only offerings. When financial aid is
applied to a course, display the discounted price with the original
price stricken. Disable the button when no price is available.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* update enrollment dialogs and buttons for enrollment mode support

- ProgramEnrollmentDialog: remove course picker, enroll in program directly;
  add certificate upsell (with Add to Cart) for "both" enrollment modes
- CourseEnrollmentDialog: add hideUpsell prop for free-only courses
- ProgramEnrollmentButton: branch on enrollmentType — free enrolls directly,
  paid goes straight to cart, both opens dialog
- CourseEnrollmentButton: branch on enrollmentType — free opens dialog with
  upsell hidden, paid goes straight to cart, both opens full dialog
- Add useCreateProgramEnrollment hook to mitxonline enrollment hooks
- Update tests for all new behaviors; remove 100x repeat from ProgramEnrollmentButton.test.tsx

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* hide enrollment upsell based on run/program enrollment modes

CourseEnrollmentDialog now derives upsell visibility from the selected
run's enrollment_modes rather than a hideUpsell prop, so the upsell
reacts dynamically as the user changes their run selection. Removes the
hideUpsell prop entirely.

ProgramEnrollmentDialog now checks program.enrollment_modes and only
renders the certificate upsell when the program offers both free and
paid options.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* make flakey test detection easier

* unify price formatting: drop .00 for whole-dollar amounts

Add formatPrice() utility that uses minimumFractionDigits:0 so whole
dollar amounts render as "$900" rather than "$900.00". Use it in program
components (ProgramEnrollmentButton, ProgramPriceRow, ProgramCertificateBox)
which previously used raw template literals. Update all affected tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: add click-action coverage for free/both enrollment modes

Add test.each covering free-only and both-modes cases that clicking
'Enroll for Free' as an authenticated user opens CourseEnrollmentDialog.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: paid enrollment loading state, error handling, and price formatting

- Show loading spinner (endIcon) while basket API calls are in flight,
  keeping button text visible and disabling the button
- Catch errors from clearBasket/addToBasket mutateAsync and surface them
  via an Alert below the button; same for free program enrollment
- Fix inconsistent price formatting in ProgramCertificateUpsell: use
  formatPrice() instead of template literal to drop unnecessary .00
- Collapse redundant free/both branches in CourseEnrollmentButton click handler
- Add tests for loading and error states in both enrollment buttons

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* unify clear+add basket logic

* make onReset optional

* fix copy

* add a test covering successful enrollment redirection

* handle errors in dialogs

* no redundant resets

* simplify: use mutate+onSuccess chaining in useReplaceBasketItem; remove redundant reset calls and try/catch at call sites

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* reset during combined mutation, remove a comment

* remove unncessary comparison

* fix a rebasing issue

* disable enrollment button when enrollment_modes is empty

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* show cents except on button

* open enrollment dialog for multi-run or upsell; direct enroll otherwise

- Open dialog when enrollmentType is "both" (upsell) or multiple enrollable runs exist
- For paid-only + single run, go directly to checkout (unchanged)
- For free-only + single run, enroll directly and redirect to dashboard home

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* be defensive in dialog: hide/disable upsell+free button for bad enrollment data

- confirmText now requires enrollmentType to be "free" or "both" (not just not-paid),
  so it's hidden for "none" (bad data) as well as "paid"
- isFreeOnly now true for any type that isn't "paid" or "both", so the upsell
  is disabled for "none" in addition to "free"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* update mitxonline-api-axios to semver release

* remove isFreeOnly from upsell props

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* support video_url on product pages and render it in place of the sidebar image if needed

* support regular embed links that go in an iframe

* add an mitxonline path friendly placeholder image for now to fix the broken image issue

* remove redundant re-export

* give a descriptive title to the sidebar video

* add some basic tests for convertToEmbedUrl
Co-authored-by: Ahtesham Quraish <ahtesham.quraish@192.168.10.4>
mbertrand and others added 21 commits March 20, 2026 14:02
* fix: preserve requirement tree ordering for program courses

Courses in ProgramEnrollmentDisplay were shown in API return order
rather than the order defined in the program's requirement tree.

Closes mitodl/hq#10528

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: use Map for coursesById lookup in program enrollment display

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Apply suggestions from code review

Co-authored-by: Chris Chudzicki <christopher.chudzicki@gmail.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…3051)

* feat: make programPageView display_mode-aware

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add display_mode to program test factories

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: create ProgramAsCourseSummary component (no requirements row)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: update programPageView call sites to pass program object

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: create ProgramAsCourseInfoBox component

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: redirect /programs/ to /courses/p/ when display_mode is course

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: create ProgramAsCoursePage client component

Adds ProgramAsCoursePage which renders a program using course-page
layout (Course breadcrumb, Course productNoun, no RequirementsSection,
no program_type tag) backed by programsQueries and pagesQueries.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat: add /courses/p/[readable_id] server route for program-as-course

Creates the Next.js server component at /courses/p/[readable_id] that
fetches program data, redirects to the program page if display_mode is
not "course", and renders ProgramAsCoursePage.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: add ModulesSection to ProgramAsCoursePage

Displays program courses as simple title-only module cards using
req_tree data. Handles single root (flat list with intro count text)
and multiple root (subsection h3 headings) layouts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix some spacing

* fix some spacing

* fix: align ProgramAsCoursePage with rebased ProductPageTemplate

- Remove tags prop (removed from ProductPageTemplate after rebase)
- Add required enrollmentAction prop with ProgramEnrollmentButton
- Fix imageSrc to use feature_image_src || fallback (consistent with other pages)
- Pass courses to InfoBox so PaceRow renders correctly
- Add PaceRow test coverage with course data
- Update tag test to assert no platform/program_type tags
- Add TODO comment for module card heading accessibility
- Remove unused type import

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* tweak programPageView signature, add some comments

* refactor: code review improvements for program-as-course feature

- Remove duplicate useQuery calls in ModulesSection and RequirementsSection
  by passing courses/isLoading as props from parent components
- Replace local keyBy utility with lodash import (consistent with codebase)
- Filter display_mode="course" programs from ProgramBundleUpsell
- Pass program.display_mode in redirect instead of hardcoded null
- Wrap multi-root module subsections in <section> elements for accessibility
- Move ProgramAsCourseSummary into ProductSummary.tsx alongside siblings
- Remove redundant InfoBoxProgramAsCourse tests (covered by page tests)
- Add course title verification to multi-root modules test
- Add test for unknown display_mode fallback behavior

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: use skeleton fallback for courses loading OR error states

Prevents showing "This course has N modules" with an empty list when
the courses query fails.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: clarify multi-root modules test is a fallback behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: verify display_mode=course programs excluded from bundle upsell

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: improve test quality — reduce hard-coding, add heading outline test

- Remove unnecessary hard-coded page override in ProgramAsCourseSummary test
- Use factory-generated title instead of hard-coded "Data Science" in ProgramBundleUpsell test
- Remove vacuous "no platform tags" test (component never renders tags)
- Add RequirementTreeBuilder.requirements() to keep req_tree and requirements in sync
- Convert programPageView multi-assertion test to test.each
- Add heading-outline accessibility test for ProgramAsCoursePage

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* test: consolidate redundant Course-label tests, add makeProgramAsCourse helper

The headings test already asserts all "Course" vs "Program" labels, so
remove three duplicative tests. Add a makeProgramAsCourse factory helper
to reduce display_mode boilerplate across the remaining tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* refactor: derive course IDs from req_tree instead of requirements

The coursesForProgram query read program.requirements to determine which
courses to fetch, while the UI rendered module structure from req_tree.
This introduced a correctness gap if the two ever diverged. Now both
pages extract course IDs via getCourseIdsFromReqTree and call coursesList
directly, making req_tree the single source of truth.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix small ts error

* reword two comments

* fix enrolled button styling

* address PR review: add pchar test coverage, use PageProps type

- Use realistic readable_id with pchar characters (: and +) in
  programPageView tests to verify they aren't percent-encoded
- Replace bespoke Props type with PageProps<"/courses/p/[readable_id]">
  for consistency with other route files

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* use "course" language in ProgramEnrollmentDialog on ProgramAsCoursePage

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
* Update everything to use the v3 enrollments endpoint

* upgrade api temporarily to branch build and use new upgrade product fields

* if a course run is passed in, get the b2b contract ID directly from the v3 run data

* fix typecheck issue with missing upgrade product props

* fix is_upgradable check

* properly handle upgrade deadline

* switch back to release api client

* fix test mock after rebase

* copilot suggestion regarding checking product id before rendering upgrade banner

* address feedback
* logic fix

* adding test

* fixing test

* move check outside of ocr method

* Revert "move check outside of ocr method"

This reverts commit ced5536.

* move check outside of ocr

* fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* catch all pdf errors

* fix test

* fix test

* fix check

* fix tests

* switch logging statements

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…#3066)

* Changing API from the regular enrollment one to the verified program-specific one when the card displays an course in a program

* formatting

* update isPending, remove stale references to createEnrollment

* fixes for other things copilot noted

* forgot to fix the call itself

* fixing a different test
Comment on lines +97 to +99
queryKey: enrollmentKeys.courseRunEnrollmentsList(),
})
},
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The useCreateVerifiedProgramEnrollment hook invalidates the courseRunEnrollmentsList cache key instead of the correct programEnrollmentsList key after creating a program enrollment.
Severity: HIGH

Suggested Fix

In the onSettled callback of the useCreateVerifiedProgramEnrollment hook, change the queryKey for queryClient.invalidateQueries from enrollmentKeys.courseRunEnrollmentsList() to enrollmentKeys.programEnrollmentsList() to ensure the correct cache is invalidated.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: frontends/api/src/mitxonline/hooks/enrollment/index.ts#L97-L99

Potential issue: The `useCreateVerifiedProgramEnrollment` hook is designed to create a
verified program enrollment. However, in its `onSettled` callback, it incorrectly
invalidates the TanStack Query cache for course run enrollments
(`enrollmentKeys.courseRunEnrollmentsList()`) instead of program enrollments
(`enrollmentKeys.programEnrollmentsList()`). This mismatch means that after a user
successfully enrolls in a program, the UI component displaying their program enrollments
will not update, showing stale data until a full page refresh.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

👍

@feoh feoh merged commit c7b14a6 into main Mar 20, 2026
14 checks passed
@feoh feoh deleted the feat/mitol-django-observability branch March 20, 2026 19:47
This was referenced Mar 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.