Skip to content

normalize MessageId filters for event subscriptions#57

Merged
raja-gade merged 1 commit intointeg_sp8from
redfish_event_listoner
Mar 31, 2026
Merged

normalize MessageId filters for event subscriptions#57
raja-gade merged 1 commit intointeg_sp8from
redfish_event_listoner

Conversation

@raja-gade
Copy link
Copy Markdown
Collaborator

Allow MessageIds to be specified with or without registry prefixes and store them in fully-qualified form for consistent event filtering.

Always cache the Redfish event log file on watcher startup.

Test:
Redfish Event subscription is working fine for the subscribed MessageIDs

Copy link
Copy Markdown
Collaborator

@supven01 supven01 left a comment

Choose a reason for hiding this comment

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

@copilot - Can you review changes?

Copy link
Copy Markdown

Copilot AI commented Mar 30, 2026

@copilot - Can you review changes?

Here is my review of the changes in this PR:

redfish-core/lib/event_service.hpp

✅ The <format> header is correctly added since std::format is now used.

Core fix: Previously, bare message keys like "PostComplete" were stored as-is, but eventMatchesFilter() always looks for "Registry.MessageKey" format (e.g., "OpenBMC.PostComplete"). This caused MessageId filtering to silently fail for all subscriptions. The change fixes this by normalizing before storing.

✅ Both input formats are correctly handled:

  • Bare key: "PostComplete" → stored as "OpenBMC.PostComplete"
  • Prefixed key: "OpenBMC.PostComplete" → stored as "OpenBMC.PostComplete"

✅ The break after adding to qualifiedMsgIds correctly prevents duplicate entries if the same message key appears in multiple registries.

⚠️ Minor: The else branch at line 643 (registryPrefix = subValue->userSub->registryPrefixes) is unreachable when processing MessageIds, because the validation at line 375-383 already enforces that RegistryPrefixes and MessageIds cannot both be non-empty in the same request.

⚠️ Note: Full 4-part Redfish MessageId format ("OpenBMC.0.1.PostComplete") is not handled — the first dot split would give idMsgKey = "0.1.PostComplete" which won't match registry entries. This is a pre-existing limitation (original code also didn't support it), so not a regression.

⚠️ Persisted data: Existing subscriptions stored on disk with old bare-key format will not be re-normalized on load and will continue to have non-matching MessageIds. Since the original code was already broken for this case, it is not a regression. For production deployments, subscriptions would need to be recreated after this update.


redfish-core/src/filesystem_log_watcher.cpp

Correct: Removing the if (redfishLogFilePosition != 0) guard and always calling cacheRedfishLogFile() on startup prevents historical log entries from being replayed as new events to subscribers when bmcweb restarts. This is the right behavior.


Overall: The changes are correct and fix a real bug where MessageId filtering was silently not working. No critical issues identified.

Copilot AI requested a review from supven01 March 30, 2026 16:54
Allow MessageIds to be specified with or without registry prefixes and
store them in fully-qualified form for consistent event filtering.

Always cache the Redfish event log file on watcher startup.

Test:
Redfish Event subscription is working fine for the subscribed
MessageIDs

Signed-off-by: Raja Sekhar Reddy Gade <RajaSekharReddy.Gade@amd.com>
@raja-gade raja-gade force-pushed the redfish_event_listoner branch from f137c1d to 45cc80b Compare March 31, 2026 15:11
@raja-gade
Copy link
Copy Markdown
Collaborator Author

@copilot - Can you review changes?

Addressed the feedback from copilot

@raja-gade raja-gade merged commit bbb0c49 into integ_sp8 Mar 31, 2026
1 check passed
@raja-gade raja-gade deleted the redfish_event_listoner branch March 31, 2026 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants