feat: enhance notification system and migrate to pubsub/v2 to fix CI#642
Open
nothing0012 wants to merge 18 commits intomainfrom
Open
feat: enhance notification system and migrate to pubsub/v2 to fix CI#642nothing0012 wants to merge 18 commits intomainfrom
nothing0012 wants to merge 18 commits intomainfrom
Conversation
nothing0012
changed the title
marceloboeira
approved these changes
Dec 26, 2025
zhouzhuojie
force-pushed
the
zz/add-notif-1
branch
from
80d6654 to
a603682
Compare
- Included flag description and pre/post change values in notifications - Implemented privacy-first, opt-in JSON diffing (FLAGR_NOTIFICATION_DETAILED_DIFF_ENABLED) - Added automatic pretty-printing for JSON diffs to improve readability - Refactored notification package for better testability (removed public init, added SetNotifier) - Added handler-level integration tests for notifications - Upgraded cloud.google.com/go/pubsub to v2 to resolve deprecation warnings - Fixed various test stability issues (DB transaction ordering, nil notifier panics)
Updated slack.go and email.go to use the defined Operation constants instead of string literals for better consistency and type safety.
Refactored entity.SaveFlagSnapshot to accept notification.Operation instead of a string, improving type safety and consistency across handler calls.
…est coverage - Renamed hook.go to dispatch.go to better reflect its purpose - Add exponential backoff retry logic for HTTP providers (email, webhook) - Add configurable retry parameters (max retries, base delay, max delay) - Add startup validation for notification configuration - Add concurrency limiting with semaphore (default 100) - Add detailed metrics for notification success/failure - Update SaveFlagSnapshot to handle soft-deleted flags correctly - Add missing notifications for DeleteFlag, RestoreFlag, and SetFlagEnabledState - Fix OperationUpdate constant usage in variant deletion - Add comprehensive tests for new notification scenarios - Update documentation with new features and configuration options This provides a more robust and observable notification system with better error handling and test coverage.
…private API) Simplify public API by making SendNotification private and keeping only SendFlagNotification as the public interface. This clarifies that the notification system currently only supports flag entities and prevents misuse. This is a non-breaking refactoring - all internal callers already use SendFlagNotification.
zhouzhuojie
force-pushed
the
zz/add-notif-1
branch
from
a05edd2 to
007a828
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR enhances the Flagr notification system with richer context, adds generic webhook support, implements retry logic with exponential backoff, adds comprehensive validation, and stabilizes the CI pipeline through a modern dependency upgrade.
Major Enhancements:
Notification System Improvements
FLAGR_NOTIFICATION_MAX_RETRIES(default: 3) - Maximum retry attemptsFLAGR_NOTIFICATION_RETRY_BASE(default: 1s) - Base delay for backoffFLAGR_NOTIFICATION_RETRY_MAX(default: 10s) - Maximum delay between retriesnotification.sent) tagged with provider, operation, entity_type, and status for monitoring.ValidateConfig()call to validate notification configuration at startup and log warnings if misconfigured.Bug Fixes & Reliability
sendNotificationwhere notifiers were captured inside the goroutine. Now captures notifiers before spawning the goroutine to prevent test pollution between test runs.SaveFlagSnapshotto properly handle soft-deleted flags usingUnscoped()queries, ensuring notifications work correctly for delete/restore operations.DeleteFlag(operation:delete)RestoreFlag(operation:restore)SetFlagEnabledState(operation:update)"update"to propernotification.OperationUpdateconstant in variant deletion.Code Quality & Testing
hook.go→dispatch.goto better reflect its responsibility.Name()method toNotifierinterface for better observability and metrics.Documentation Updates
docs/flagr_notifications.mdwith new features, retry configuration, concurrency limits, and important notes about asynchronous delivery.CI & Dependency Migration
cloud.google.com/go/pubsubfromv1tov2to resolve deprecation warnings.Topicusage toPublisherin both producer logic and tests.SA1019lint warnings which were blocking the CI pipeline.Motivation and Context
The notification system needed better reliability, observability, and configurability for production use. The retry logic ensures transient failures don't result in lost notifications. The validation and metrics provide operational visibility. The bug fixes ensure all flag operations trigger appropriate notifications, even for soft-deleted entities.
How Has This Been Tested?
make cipassed with 0 lint issues and 0 test failures.Types of changes
Checklist:
make ci).