Add virtual delegates for frequently used event stream API columns #22398

jrafanie · 2023-03-09T22:49:55Z

See client UI code in:
ManageIQ/manageiq-ui-classic#8562
ManageIQ/manageiq-ui-classic#8642

This is a followup to #22361

Previously, the UI needed to run an API request including the vm, host, and
ext management system and all of their data because sometimes the event
stream didn't have a vm, host or ems so the UI code would need to add
conditional logic as vm.name, host.name and ext_mangement_system.name
would be empty for some entries and exist for others. See [1].

We can add virtual delegates to always return a name or null if the relationship
doesn't exist or the name is null. This saves us lots of data if we're only
using a single column from each relationship.

For a specific API call with proper timestamp filtering so we only return tens
of thousands of event_streams:

"name": "event_streams",
"count": 299925,
"subcount": 1000,
"subquery_count": 39487,
"pages": 40,

Before:

http://localhost:3000/api/event_streams?limit=5000&offset=0&expand=resources&attributes=group,group_level,group_name,id,event_type,ems_id,type,timestamp,created_on,host,source,message,vm,ext_management_system&filter[]=ems_id=2&filter[]=type=EmsEvent&filter[]=group=[configuration,other]&filter[]=group_level=[warning,detail,critical]&filter[]=timestamp%3E2022-10-01T22:19:39.148Z&filter[]=timestamp%3C2022-10-5T22:19:39.148Z

20.07 s
2.59 MB

After:

http://localhost:3000/api/event_streams?limit=5000&offset=0&expand=resources&attributes=group,group_level,group_name,id,event_type,ems_id,type,timestamp,created_on,host_name,source,message,vm_or_template_name,ext_management_system_name&filter[]=ems_id=2&filter[]=type=EmsEvent&filter[]=group=[configuration,other]&filter[]=group_level=[warning,detail,critical]&filter[]=timestamp%3E2022-10-01T22:19:39.148Z&filter[]=timestamp%3C2022-10-5T22:19:39.148Z

(replacing ext_management_system, host, and vm with ext_management_system_name,
host_name, and vm_or_template_name)

15.89 s
391 KB

[1] If you use association.method format: vm_or_template.name,
ext_management_system.name, host.name, you are missing sections if they're nil,
leading to client code needing to have conditional logic:

{
    "href": "http://localhost:3000/api/event_streams/20409",
    "id": "20409",
    "event_type": "INVALID_URI",
    "ems_id": "2",
    "type": "EmsEvent",
    "timestamp": "2022-10-03T13:05:31Z",
    "created_on": "2022-10-03T12:35:09Z",
    "source": "IBM_POWER_HMC",
    "message": null,
    "group": "other",
    "group_level": "detail",
    "group_name": "Other",
    "vm_or_template": {
        "name": "vm1"
    },
    "ext_management_system": {
        "name": "ems1"
    }
},
{
    "href": "http://localhost:3000/api/event_streams/20410",
    "id": "20410",
    "event_type": "INVALID_URI",
    "ems_id": "2",
    "type": "EmsEvent",
    "timestamp": "2022-10-03T13:05:51Z",
    "created_on": "2022-10-03T12:35:24Z",
    "source": "IBM_POWER_HMC",
    "message": null,
    "group": "other",
    "group_level": "detail",
    "group_name": "Other",
    "ext_management_system": {
        "name": "ems1"
    }
},

If you use virtual columns, it's flat and easy to use:

{
    "href": "http://localhost:3000/api/event_streams/20409",
    "id": "20409",
    "event_type": "INVALID_URI",
    "ems_id": "2",
    "type": "EmsEvent",
    "timestamp": "2022-10-03T13:05:31Z",
    "created_on": "2022-10-03T12:35:09Z",
    "host_name": null,
    "source": "IBM_POWER_HMC",
    "message": null,
    "group": "other",
    "group_level": "detail",
    "group_name": "Other",
    "vm_or_template_name": "vm1",
    "ext_management_system_name": "ems1"
},
{
    "href": "http://localhost:3000/api/event_streams/20410",
    "id": "20410",
    "event_type": "INVALID_URI",
    "ems_id": "2",
    "type": "EmsEvent",
    "timestamp": "2022-10-03T13:05:51Z",
    "created_on": "2022-10-03T12:35:24Z",
    "host_name": null,
    "source": "IBM_POWER_HMC",
    "message": null,
    "group": "other",
    "group_level": "detail",
    "group_name": "Other",
    "vm_or_template_name": null,
    "ext_management_system_name": "ems1"
},

jrafanie · 2023-03-09T22:52:42Z

@MelsHyrule this PR should allow you to replace ext_management_system, host, and vm in your API call with ext_management_system_name, host_name, and vm_or_template_name. It's a small optimization to bring back much less data. You would then always have a key ext_management_system_name, host_name, and vm_or_template_name, but they could be null values.

This is a followup to an issue I saw in #22361

kbrock

could you try this out and see how it works?

I'm hoping it will be able to bring back everything in a single table and not do the includes over to ems, host, and vms

app/models/event_stream.rb

kbrock · 2023-03-10T01:25:24Z

A. I'm hoping this will be quicker
B. Not sure if this will kick in the rbac virtual attribute optimization, that may buy us a little too.

Fryguy · 2023-03-10T23:16:54Z

+1 for virtual delegates. @jrafanie you should see even more time savings

jrafanie · 2023-03-13T18:36:56Z

A. I'm hoping this will be quicker
B. Not sure if this will kick in the rbac virtual attribute optimization, that may buy us a little too.

Thanks. Sorry, the "stale" notifications flooded me so I dropped all notifications and missed this.

I'll take a look.

jrafanie · 2023-03-13T18:52:02Z

Using virtual delegates:

15.89 s
391.3 KB

That's about 3-4 seconds faster.

See client UI code in: ManageIQ/manageiq-ui-classic#8562 ManageIQ/manageiq-ui-classic#8642 This is a followup to ManageIQ#22361 Previously, the UI needed to run an API request including the `vm`, `host`, and `ext management system` and all of their data because sometimes the event stream didn't have a `vm`, `host` or `ems` so the UI code would need to add conditional logic as `vm.name`, `host.name` and `ext_mangement_system.name` would be empty for some entries and exist for others. See [1]. We can add virtual delegates to always return a name or null if the relationship doesn't exist or the name is null. This saves us lots of data if we're only using a single column from each relationship. For a specific API call with proper timestamp filtering so we only return tens of thousands of event_streams: "name": "event_streams", "count": 299925, "subcount": 1000, "subquery_count": 39487, "pages": 40, Before: ``` http://localhost:3000/api/event_streams?limit=5000&offset=0&expand=resources&attributes=group,group_level,group_name,id,event_type,ems_id,type,timestamp,created_on,host,source,message,vm,ext_management_system&filter[]=ems_id=2&filter[]=type=EmsEvent&filter[]=group=[configuration,other]&filter[]=group_level=[warning,detail,critical]&filter[]=timestamp%3E2022-10-01T22:19:39.148Z&filter[]=timestamp%3C2022-10-5T22:19:39.148Z 20.07 s 2.59 MB ``` After: ``` http://localhost:3000/api/event_streams?limit=5000&offset=0&expand=resources&attributes=group,group_level,group_name,id,event_type,ems_id,type,timestamp,created_on,host_name,source,message,vm_or_template_name,ext_management_system_name&filter[]=ems_id=2&filter[]=type=EmsEvent&filter[]=group=[configuration,other]&filter[]=group_level=[warning,detail,critical]&filter[]=timestamp%3E2022-10-01T22:19:39.148Z&filter[]=timestamp%3C2022-10-5T22:19:39.148Z (replacing ext_management_system, host, and vm with ext_management_system_name, host_name, and vm_or_template_name) 15.89 s 391 KB ``` [1] If you use association.method format: vm_or_template.name, ext_management_system.name, host.name, you are missing sections if they're nil, leading to client code needing to have conditional logic: ``` { "href": "http://localhost:3000/api/event_streams/20409", "id": "20409", "event_type": "INVALID_URI", "ems_id": "2", "type": "EmsEvent", "timestamp": "2022-10-03T13:05:31Z", "created_on": "2022-10-03T12:35:09Z", "source": "IBM_POWER_HMC", "message": null, "group": "other", "group_level": "detail", "group_name": "Other", "vm_or_template": { "name": "vm1" }, "ext_management_system": { "name": "ems1" } }, { "href": "http://localhost:3000/api/event_streams/20410", "id": "20410", "event_type": "INVALID_URI", "ems_id": "2", "type": "EmsEvent", "timestamp": "2022-10-03T13:05:51Z", "created_on": "2022-10-03T12:35:24Z", "source": "IBM_POWER_HMC", "message": null, "group": "other", "group_level": "detail", "group_name": "Other", "ext_management_system": { "name": "ems1" } }, ``` If you use virtual columns, it's flat and easy to use: ``` { "href": "http://localhost:3000/api/event_streams/20409", "id": "20409", "event_type": "INVALID_URI", "ems_id": "2", "type": "EmsEvent", "timestamp": "2022-10-03T13:05:31Z", "created_on": "2022-10-03T12:35:09Z", "host_name": null, "source": "IBM_POWER_HMC", "message": null, "group": "other", "group_level": "detail", "group_name": "Other", "vm_or_template_name": "vm1", "ext_management_system_name": "ems1" }, { "href": "http://localhost:3000/api/event_streams/20410", "id": "20410", "event_type": "INVALID_URI", "ems_id": "2", "type": "EmsEvent", "timestamp": "2022-10-03T13:05:51Z", "created_on": "2022-10-03T12:35:24Z", "host_name": null, "source": "IBM_POWER_HMC", "message": null, "group": "other", "group_level": "detail", "group_name": "Other", "vm_or_template_name": null, "ext_management_system_name": "ems1" }, ```

jrafanie · 2023-03-13T19:00:41Z

Using virtual delegates:
15.89 s
391.3 KB
That's about 3-4 seconds faster.

vs. before PR:

20.43 s
2.59 MB

vs. manual virtual columns

20.10 s
375.04 KB

jrafanie · 2023-03-13T19:03:31Z

I've pushed the suggestion to use virtual delegates. Now, it's faster and much smaller response. Thanks @kbrock

miq-bot · 2023-03-13T19:07:32Z

Checked commit jrafanie@002627a with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 🍰

kbrock · 2023-03-14T05:15:07Z

what is the strategy? merge while red (since the failure is unrelated) or wait?

jrafanie · 2023-03-14T16:16:39Z

what is the strategy? merge while red (since the failure is unrelated) or wait?

thanks, I kicked it... this isn't urgent... let's make sure it's 💚 first

jrafanie · 2023-03-14T18:02:36Z

Ok, now this is 💚

jrafanie · 2023-03-14T18:04:34Z

Adding backport for this. It's such a minor change. We don't need it now but if we need to fix the UI to use this more efficient mechanism, this PR should be already there.

Fryguy · 2023-03-30T16:20:09Z

Backported to petrosian in commit 4656db5.

commit 4656db5412ebf2eb5f9885d7ec8315683c88276e
Author: Keenan Brock <keenan@thebrocks.net>
Date:   Tue Mar 14 15:46:33 2023 -0400

    Merge pull request #22398 from jrafanie/add_timeline_event_stream_virtual_columns_for_name
    
    Add virtual delegates for frequently used event stream API columns
    
    (cherry picked from commit 157507ad04bf0914c583d9e3474335b6e7a74a3e)

…tual_columns_for_name Add virtual delegates for frequently used event stream API columns (cherry picked from commit 157507a)

jrafanie requested a review from agrare as a code owner March 9, 2023 22:49

jrafanie added enhancement core/events labels Mar 9, 2023

jrafanie mentioned this pull request Mar 9, 2023

Fix event stream filtering so we can filter by ems event or miq event (policy) #22361

Merged

3 tasks

jrafanie force-pushed the add_timeline_event_stream_virtual_columns_for_name branch 2 times, most recently from 90d8dc2 to a2e32bc Compare March 9, 2023 23:01

jrafanie mentioned this pull request Mar 9, 2023

React Timeline Chart and Table Component ManageIQ/manageiq-ui-classic#8562

Merged

kbrock requested changes Mar 10, 2023

View reviewed changes

app/models/event_stream.rb Outdated Show resolved Hide resolved

app/models/event_stream.rb Outdated Show resolved Hide resolved

kbrock self-assigned this Mar 10, 2023

jrafanie force-pushed the add_timeline_event_stream_virtual_columns_for_name branch from a2e32bc to 002627a Compare March 13, 2023 19:01

jrafanie changed the title ~~Add virtual columns for frequently used event stream API columns~~ Add virtual delegates for frequently used event stream API columns Mar 13, 2023

jrafanie closed this Mar 14, 2023

jrafanie reopened this Mar 14, 2023

jrafanie added the petrosian/yes? label Mar 14, 2023

kbrock merged commit 157507a into ManageIQ:master Mar 14, 2023

jrafanie deleted the add_timeline_event_stream_virtual_columns_for_name branch March 14, 2023 19:48

Fryguy removed the petrosian/yes? label Mar 29, 2023

Fryguy added the petrosian/yes label Mar 29, 2023

Fryguy pushed a commit that referenced this pull request Mar 30, 2023

Merge pull request #22398 from jrafanie/add_timeline_event_stream_vir…

4656db5

…tual_columns_for_name Add virtual delegates for frequently used event stream API columns (cherry picked from commit 157507a)

Fryguy added petrosian/backported and removed petrosian/yes labels Mar 30, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add virtual delegates for frequently used event stream API columns #22398

Add virtual delegates for frequently used event stream API columns #22398

jrafanie commented Mar 9, 2023 •

edited

jrafanie commented Mar 9, 2023

kbrock left a comment

kbrock commented Mar 10, 2023

Fryguy commented Mar 10, 2023

jrafanie commented Mar 13, 2023

jrafanie commented Mar 13, 2023

jrafanie commented Mar 13, 2023

jrafanie commented Mar 13, 2023

miq-bot commented Mar 13, 2023

kbrock commented Mar 14, 2023

jrafanie commented Mar 14, 2023

jrafanie commented Mar 14, 2023

jrafanie commented Mar 14, 2023

Fryguy commented Mar 30, 2023

Add virtual delegates for frequently used event stream API columns #22398

Add virtual delegates for frequently used event stream API columns #22398

Conversation

jrafanie commented Mar 9, 2023 • edited

jrafanie commented Mar 9, 2023

kbrock left a comment

Choose a reason for hiding this comment

kbrock commented Mar 10, 2023

Fryguy commented Mar 10, 2023

jrafanie commented Mar 13, 2023

jrafanie commented Mar 13, 2023

jrafanie commented Mar 13, 2023

jrafanie commented Mar 13, 2023

miq-bot commented Mar 13, 2023

kbrock commented Mar 14, 2023

jrafanie commented Mar 14, 2023

jrafanie commented Mar 14, 2023

jrafanie commented Mar 14, 2023

Fryguy commented Mar 30, 2023

jrafanie commented Mar 9, 2023 •

edited