-
Notifications
You must be signed in to change notification settings - Fork 31
RHINENG-17707 add unleash check to the legacy backend #739
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
Conversation
Reviewer's GuideIntroduces an Unleash-based feature flag check for the legacy ROS backend by adding a helper in the Unleash client and gating message processing in both inventory and insights engine consumers based on UNLEASH_ROS_V2_FLAG. Sequence diagram for feature flag check in inventory events consumersequenceDiagram
participant InventoryEventsConsumer
participant UnleashClient
Note over InventoryEventsConsumer: On message received
InventoryEventsConsumer->UnleashClient: is_feature_flag_enabled(org_id, UNLEASH_ROS_V2_FLAG, prefix)
UnleashClient-->>InventoryEventsConsumer: true/false
alt Feature flag enabled
InventoryEventsConsumer->>InventoryEventsConsumer: continue (skip processing)
else Feature flag disabled
InventoryEventsConsumer->>InventoryEventsConsumer: process event
end
Sequence diagram for feature flag check in insights engine consumersequenceDiagram
participant InsightsEngineConsumer
participant UnleashClient
Note over InsightsEngineConsumer: On message received
InsightsEngineConsumer->UnleashClient: is_feature_flag_enabled(org_id, UNLEASH_ROS_V2_FLAG, prefix)
UnleashClient-->>InsightsEngineConsumer: true/false
alt Feature flag enabled
InsightsEngineConsumer->>InsightsEngineConsumer: continue (skip processing)
else Feature flag disabled
InsightsEngineConsumer->>InsightsEngineConsumer: handle_msg(msg)
end
Class diagram for new Unleash feature flag helperclassDiagram
class UnleashClient {
+initialize_client()
+is_enabled(flag_name, context)
+is_initialized
}
class is_feature_flag_enabled {
+is_feature_flag_enabled(org_id, flag_name, service_name)
}
UnleashClient <.. is_feature_flag_enabled : uses
Class diagram for updated consumers with feature flag checkclassDiagram
class InventoryEventsConsumer {
+run()
-event_type_map
-prefix
}
class InsightsEngineConsumer {
+run()
+handle_msg(msg)
-prefix
}
class IsFeatureFlagEnabled {
+is_feature_flag_enabled(org_id, flag_name, service_name)
}
InventoryEventsConsumer ..> IsFeatureFlagEnabled : calls
InsightsEngineConsumer ..> IsFeatureFlagEnabled : calls
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- In is_feature_flag_enabled you pass service_name only for loggingβconsider including it in the unleash context (e.g. appName) or removing the unused parameter to keep the API surface minimal.
- In insights_engine_consumer you call get('org_id') which can return Noneβadd explicit handling or a default value to avoid passing a None userId into the Unleash client.
- Since youβre evaluating the same flag for each org_id across many messages, consider caching unleash_client.is_enabled results per (org_id, flag_name) during a run to reduce repeated network calls.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In is_feature_flag_enabled you pass service_name only for loggingβconsider including it in the unleash context (e.g. appName) or removing the unused parameter to keep the API surface minimal.
- In insights_engine_consumer you call get('org_id') which can return Noneβadd explicit handling or a default value to avoid passing a None userId into the Unleash client.
- Since youβre evaluating the same flag for each org_id across many messages, consider caching unleash_client.is_enabled results per (org_id, flag_name) during a run to reduce repeated network calls.
## Individual Comments
### Comment 1
<location> `ros/processor/insights_engine_consumer.py:76` </location>
<code_context>
try:
msg = json.loads(msg.value().decode("utf-8"))
+
+ org_id = msg["input"]["platform_metadata"].get('org_id')
+ if is_feature_flag_enabled(org_id, UNLEASH_ROS_V2_FLAG, self.prefix):
+ continue
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential KeyError if 'input' or 'platform_metadata' is missing in msg.
Using .get() for these keys will prevent exceptions if the message structure is unexpected or missing fields.
</issue_to_address>Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
ba68dba to
ce09690
Compare
Why do we need this change? π
Resolves RHINENG-17707
Documentation update? π
Security Checklist π
Upon raising this PR please go through RedHatInsights/secure-coding-checklist
πββοΈ Checklist π―
Additional π£
Feel free to add any other relevant details such as links, notes, screenshots, here.
Summary by Sourcery
Integrate Unleash feature flag gating into the legacy backend by adding a helper function and using it in consumers to conditionally bypass processing when the ROS V2 flag is active.
New Features: