feat(serviceableEvents): added serviceableEvents functionality - #210
feat(serviceableEvents): added serviceableEvents functionality#210SiddhantPawar03 wants to merge 11 commits into
Conversation
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
|
|
||
| # Store complete SEM payload for downstream processing/comparison | ||
| :full_data => entry, | ||
|
|
||
| :ems_id => ems_id |
There was a problem hiding this comment.
| # Store complete SEM payload for downstream processing/comparison | |
| :full_data => entry, | |
| :ems_id => ems_id | |
| :full_data => entry, # Store complete SEM payload for downstream processing/comparison | |
| :ems_id => ems_id |
| existing_map = EmsEvent | ||
| .where(:ems_id => @ems.id, :event_type => "ServiceableEvent", :source => "IBM_POWER_HMC") | ||
| .pluck(:message, :id, :full_data) | ||
| .each_with_object({}) do |(msg, id, full_data), hash| | ||
| hash[msg] = [id, full_data] | ||
| end |
There was a problem hiding this comment.
Our goal is to move event catchers out of the Rails/ActiveRecord environment in order to reduce total worker memory usage. These types of queries break that since it accesses the database directly.
There was a problem hiding this comment.
@agrare Currently, HMC REST API does not provide an option to fetch serviceable events generated within a specific time frame, hence we cannot query only the delta. Because of this, every request to the HMC serviceable events API returns the complete set of available serviceable events. To avoid storing duplicate data, we fetch the previously stored events in DB, do comparision and store only the newly generated serviceable events.
There was a problem hiding this comment.
@shashank-gowda can you give it a timestamp of the most recent event? It seems pretty inefficient to have to get the full list everytime.
There was a problem hiding this comment.
Are the events at least timestamped and returned in order? We could check the most recent event we have, and then do a simple filtering basedf on that (or even better, pass it into the API and only gets events after that time)
There was a problem hiding this comment.
@agrare @Fryguy Unfortunately, the HMC REST API does not support passing timestamp to retrieve only events generated after a given time. Every request returns the complete set of serviceable events available on the HMC. Because of this limitation, our current approach is to retrieve the full list, compare it with the events already stored in the database, and add only the newly generated events.
Once, HMC API supports querying events after a specific timestamp, we will remove the database query and reduce the overhead. Here is an example of the API we are currently using:
/rest/api/sem/ServiceableEvent/search/hmc=all
This API only accepts hmc=all, hmc=open or hmc=close as valid values. It does not support any other filters.
There was a problem hiding this comment.
Every request returns the complete set of serviceable events available on the HMC.
To clarify does that include the closed ones or only open ones? if it's only open ones that might not be as bad. I originally read this as historically every serviceable event ever (i.e. an ever growing list)
|
I recall being asked about this elsewhere, but serviceable events sound more like what we used to call "Alarms". We had that OpenShift dashboard showing actionable "alerts" (we should have called them alarms back then but we called them alerts, confusingly, and they got conflated with Policy Alerts). (ref1, ref2) Those alarms then could be assigned to a user, actioned, and could be closed when they have been worked on or when they've been marked completed on the provider side. The primary difference between alarms/serviceable events and regular events is that the alarms have "state" whereas events as stored in the ems_events table are point-in-time statements with no "state". I suggest we have a new table for these ( @agrare I thought we did this exercise, but did we ever look at the other providers to see if they have similar functionality? I think vmware used to have "alarms", but no idea about the others. |
|
Agreed I still think events should be immutable and what you have here isn't an event but something else which is why the modeling doesn't really fit |
|
I'm fine with collecting separate events for what is effectively "alarm opened" and "alarm closed" as those are point-in-time statements, but we should not be editing existing events. If we want to pursue the alarms screen for actually having a way to action these events, I'd be up for designing that. |
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
Signed-off-by: Siddhant <siddhant@dhcp-9-123-115-184.j9d-in.ibm.com>
|
@SiddhantPawar03 @shashank-gowda we can discuss the performance issues with querying every event for the EMS and possibly come up with some improvements, but the bigger design issues with this PR are being discussed by @Fryguy and myself starting #210 (comment) |
| # 2. Fetching raw XML from the HMC via +connection.fetch_serviceable_events_xml+. | ||
| # 3. Parsing each feed entry and persisting an EmsEvent record via EmsEvent.add_queue. | ||
| # | ||
| class ManageIQ::Providers::IbmPowerHmc::InfraManager::EventCatcher::ServiceableEventPoller |
There was a problem hiding this comment.
@agrare Naming-wise, I thought we already had something that looped in the core, and could be subclassed by the providers where they just implement yielding the next event.
There was a problem hiding this comment.
EventCatcher::Runner is what does event_monitor_handle.poll in the provider.
EventCatcher::Stream is commonly what this is called, they are adding a second parallel one here which I agree isn't great.
There was a problem hiding this comment.
Oh nvm - I didn't realize this code was just an extract of the low-level polling out of the outer ManageIQ::Providers::IbmPowerHmc::InfraManager::EventCatcher::Stream class.
It's confusing because this class interacts with EmsEvent which I feel it shouldn't...this class could be a pure provider interaction class that just yields event payloads, and the stream should be responsible for adding an EmsEvent for each payload (I thought the base Stream class already did that).
There was a problem hiding this comment.
EventCatcher::Stream is just commonly what hits the provider API, it doesn't have a core base class.
the EventCatcher::Runner is what you're thinking of which loops through the yielded events, parses, and queues the event.
There was a problem hiding this comment.
Right, so the ServiceableEventPoller is replacing the Stream but doing the work of the Runner. Instead the Stream should only hit the provider API (which is fine if it's extracted into the Poller class), and the core Runner should be the one hitting the EmsEvent table.
@SiddhantPawar03 Is what we're saying makes sense? The organization of the code is confusing compared to other providers. Can you readjust it accordingly?
There was a problem hiding this comment.
Right I think either separate classes like StreamUomEvents and StreamServiceableEvents or Stream#uom_events and Stream#serviceable_events would be clearer
Signed-off-by: Siddhant <siddhant@Siddhants-MacBook-Pro.local>
|
@SiddhantPawar03 Your git commits are not being associated with your GitHub Account - make sure that whatever email address you are using to make commits is in your GitHub Account Profile. |
|
Checked commits SiddhantPawar03/pmc-manageiq-providers-ibm_power_hmc@30c2427~...930b1d3 with ruby 3.3.10, rubocop 1.88.2, haml-lint 0.76.0, and yamllint 1.37.1 |
| @@ -0,0 +1,114 @@ | |||
| require "rexml/document" | |||
There was a problem hiding this comment.
Please avoid using Rexml - in the project we use Nokogiri instead.
Summary
This PR adds support for Serviceable Events collection and processing from IBM Power HMC.
The provider is enhanced to retrieve Serviceable Events from HMC and expose them through the ManageIQ inventory framework. This enables administrators to monitor hardware and system events reported by managed Power Systems directly within ManageIQ.
Changes
Benefits
Testing
Related
Related SDK changes:
@miq-bot add-label enhancement