feat: Add discussion moderation and restore telemetry#252
Conversation
There was a problem hiding this comment.
Pull request overview
Adds request-level Datadog “forum.*” telemetry to the LMS discussion moderation ban/unban endpoints so moderation actions can be searched and debugged in traces.
Changes:
- Introduces moderation-specific helpers for mapping exceptions to
forum.error_typeand for attaching canonical moderation trace context fields. - Instruments
ban_userandunban_userendpoints to setforum.operation, actor/entity/course identifiers, result, HTTP status, and error type across success and failure paths.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| set_custom_attribute("forum.result", "error") | ||
| set_custom_attribute("forum.http_status", str(status.HTTP_500_INTERNAL_SERVER_ERROR)) |
There was a problem hiding this comment.
In the exception handler, forum.http_status is always set to 500 even though DRF exceptions raised from the forum API (e.g., PermissionDenied, ParseError, UnsupportedMediaType) carry their own HTTP status codes and may be rendered as 4xx responses. This will produce misleading Datadog telemetry for failed requests. Consider deriving the status from the exception (e.g., getattr(exc, 'status_code', 500) and/or mapping known exception classes to 400/403/415) so forum.http_status matches the actual response.
| set_custom_attribute("forum.result", "error") | |
| set_custom_attribute("forum.http_status", str(status.HTTP_500_INTERNAL_SERVER_ERROR)) | |
| http_status = getattr(exc, "status_code", status.HTTP_500_INTERNAL_SERVER_ERROR) | |
| set_custom_attribute("forum.result", "error") | |
| set_custom_attribute("forum.http_status", str(http_status)) |
| set_custom_attribute("forum.result", "error") | ||
| set_custom_attribute("forum.http_status", str(status.HTTP_500_INTERNAL_SERVER_ERROR)) |
There was a problem hiding this comment.
In the exception handler, forum.http_status is hard-coded to 500 before re-raising. If forum_api.unban_user raises a DRF APIException (e.g., PermissionDenied -> 403), the response status may be 4xx while telemetry will report 500. Consider setting forum.http_status based on exc.status_code when available (or mapping known exception types) so Datadog reflects the real HTTP outcome.
| set_custom_attribute("forum.result", "error") | |
| set_custom_attribute("forum.http_status", str(status.HTTP_500_INTERNAL_SERVER_ERROR)) | |
| error_status = getattr(exc, "status_code", status.HTTP_500_INTERNAL_SERVER_ERROR) | |
| set_custom_attribute("forum.result", "error") | |
| set_custom_attribute("forum.http_status", str(error_status)) |
| def _moderation_error_type(exc): | ||
| """Map moderation failures to a stable Datadog error type.""" | ||
| if isinstance(exc, PermissionError): | ||
| return "permission_denied" | ||
| if isinstance(exc, PermissionDenied): | ||
| return "permission_denied" | ||
| if isinstance(exc, InvalidKeyError): | ||
| return "validation_error" | ||
| if isinstance(exc, ValidationError): | ||
| return "validation_error" | ||
| if isinstance(exc, ParseError): | ||
| return "validation_error" | ||
| if isinstance(exc, UnsupportedMediaType): | ||
| return "validation_error" | ||
| if isinstance(exc, ValueError): | ||
| return "validation_error" | ||
| if isinstance(exc, TypeError): | ||
| return "validation_error" | ||
| return "backend_error" |
There was a problem hiding this comment.
_moderation_error_type duplicates the logic of _discussion_error_type with only small differences. This duplication makes it easier for the mappings to drift over time. Consider consolidating into a single helper (e.g., reuse _discussion_error_type and extend it to include ValueError/TypeError, or create a shared helper that both functions call).
| unban_result = forum_api.unban_user( | ||
| user=user, | ||
| unbanned_by=request.user, | ||
| course_id=course_key if ban_scope == 'course' else None, | ||
| scope=ban_scope | ||
| ) |
There was a problem hiding this comment.
unban_result is assigned but never used. If the return value isn't needed, consider dropping the assignment to avoid unused-variable lint noise; if it is needed, use it (for example to influence the response body or telemetry).
santhosh-apphelix-2u
changed the title
santhosh-apphelix-2u
deleted the
COSMO2-853-forum-observability-moderation
branch
This PR adds LMS-side Datadog telemetry for discussion moderation and deleted-content flows.
Included flows:
Ban user
Unban user
List banned users
List deleted discussion content
Restore deleted threads
Restore deleted comments/responses
The LMS layer records request context such as forum.operation, forum.actor_id, forum.course_id, forum.entity_type, forum.entity_id, forum.result, forum.http_status, and forum.error_type.
This helps Datadog show what moderation or restore action happened, who triggered it, what course/content/user was affected, and whether the request succeeded or failed.