change: [M3-8450] - Optimize event query timeframe: Reduce historical data fetch from 90 days to 7 days

[jaalah-akamai]

Description 📝

This change optimizes Cloud Manager's event querying by reducing the historical data range. Specifically, it limits the event query timeframe from the current 90 days to a more focused 7-day period. This adjustment aims to improve query performance and reduce unnecessary data retrieval while still providing relevant recent event information.

Changes 🔄

• Previously we fetched the first 100 events but returned almost 500 results/records. These events pull from the last 90 days. Now we're returning events from the last 7 days up to a max of 25 results per query.

Target release date 🗓️

8/19

Preview 📷

Before	After

How to test 🧪

Note

We are working to have this fully covered via E2E tests in an upcoming PR cc: @jdamore-linode

Prerequisites

• Checkout branch and observe you network tab

Verification steps

• Go to http://localhost:3000/events
• Observe that initial load returns at most 25 events
• As you scroll observe subsequent queries fetch next 25
• Ensure there aren't any redundant events
• Ensure you're able to reach the end of your events list (90 days worth of events)

As an Author I have considered 🤔

Check all that apply

• 👀 Doing a self review
• ❔ Our contribution guidelines
• 🤏 Splitting feature into small PRs
• ➕ Adding a changeset
• 🧪 Providing/Improving test coverage
• 🔐 Removing all sensitive information from the code and PR description
• 🚩 Using a feature flag to protect the release
• 👣 Providing comprehensive reproduction steps
• 📑 Providing or updating our documentation
• 🕛 Scheduling a pair reviewing session
• 📱 Providing mobile support
• ♿ Providing accessibility support

[github-actions]

Coverage Report: ✅
Base Coverage: 86.04%
Current Coverage: 86.04%

[bnussman-akamai]

This seems to be working better with the changes from 37e29a3

If we proceed with ordering events by created instead of id, I believe we'll need to assess the implications this has on useMarkEventsAsSeen and generatePollingFilter to account for the fact that we order by created instead of id. I'm realizing this because I have some events that are not are not marked as seen, but they should be.

Screen.Recording.2024-08-15.at.12.39.32.PM.mov

The API appears to use id rather than created to compute seen, so we might have some issues making this work as expected. Going to keep thinking 🧠

[hkhalil-akamai]

The API appears to use id rather than created to compute seen, so we might have some issues making this work as expected. Going to keep thinking 🧠

We may have to loop through all events and mark them as seen individually. We may be able to optimize this by keeping track of the highest id we have marked as seen and avoiding sending requests for lower ids.

Edit: realized we can accomplish the same with a single request by marking the highest id as seen.

[jdamore-linode]

@bnussman-akamai Just calling out that the limit filter went away with the most recent commit -- was that intended?

[bnussman-akamai]

I did remove it intentionally. We could add it back, but I don't think it's necessary because most of the backend performance improvements come from the created filter. The backed will use a limit (page size) of 100 here, which should be okay

[jaalah-akamai]

@bnussman-akamai Updates are working very nice. Both caches stay in sync for in-progress events and initial query fetches < 100. I think that part of our work is sound 🔥

@jdamore-linode Are we just trying to get Makes initial fetch to events endpoint test working?

[hkhalil-akamai]

Verified behavior of:

• Initially, last 7 days of events are fetched
• When opening the Events list, all events are fetched, with pagination
• Events are ordered by id, descending
• Events polling works as expected, updating the useEventsInfinite and useInitialEvents cache
• Polling accelerates during in-progress events

All enabled Cypress tests are passing, but some are skipped and those may not pass (including one that tests for out-of-order IDs).

Great work @jdamore-linode @bnussman-akamai! 👏🏽

[hkhalil-akamai]

Could this test fail if the code is executing slow enough for there to be a delay between the timestamps? (cc @jdamore-linode)

[jaalah-akamai]

Yea this is failing for me, we should add some tolerance or an acceptable range of 1 day instead of comparing the stamps directly

[jdamore-linode]

It's not comparing the stamps directly, it's omitting the time and only asserting that the date matches what Cloud is filtering for.

Mind posting the error you're seeing, @jaalah-akamai? A little wrapped up right now so I can't pull the changes to try to reproduce.

[jaalah-akamai]

AssertionError: expected '{"action":{"+neq":"profile_update"},"+order":"desc","+order_by":"id","created":{"+gt":"2024-08-16T02:35:03"}}' to include '"created":{"+gt":"2024-08-15'

[jaalah-akamai]

I was able to get it to pass last night after debugging, but it's more verbose than I like and you may have a better solution.

// We need to narrow whether the input is a string or an array of strings
const parseFilters = (input: string | string[]): Record<string, any> => {
  if (Array.isArray(input)) {
    return parseArrayOfFilters(input);
  }
  return JSON.parse(input);
};

// Parse the array of strings and combine them into a single object.
const parseArrayOfFilters = (filterArray: string[]): Record<string, any> => {
  return filterArray.reduce((combinedFilters, filterString) => {
    const parsedFilter = JSON.parse(filterString);
    return {
      ...combinedFilters,
      ...parsedFilter
    };
  }, {});
};

it('Makes initial fetch to events endpoint', () => {
    mockGetEvents([]).as('getEvents');
    cy.visitWithLogin('/');
    cy.wait('@getEvents').then((xhr) => {
      const filters = xhr.request.headers['x-filter'];
      const lastWeekTimestamp = DateTime.now().minus({ weeks: 1 });

      /*
       * Confirm that initial fetch request contains filters to achieve
       * each of the following behaviors:
       *
       * - Exclude `profile_update` events.
       * - Sort events by their created date.
       * - Only retrieve events created within the past week.
       */

      const parsedFilters = parseFilters(filters);

      // Check for profile_update exclusion
      expect(parsedFilters.action?.['+neq']).to.equal('profile_update');
      expect(parsedFilters['+order_by']).to.equal('id');

      // Check for date filter
      const filterDate = DateTime.fromISO(parsedFilters.created['+gt']);

      // Check if the filter date is within the last week
      const daysDifference = filterDate.diff(lastWeekTimestamp, 'days').days;
      expect(daysDifference).to.be.closeTo(0, 1);
    });
  });

[jdamore-linode]

That's a head scratcher for sure.

Thanks for the snippet @jaalah-akamai! I'll try to find some time this afternoon to check all this out

[hkhalil-akamai]

Are we skipping this test because it's not passing currently?

[hkhalil-akamai]

It seems like the events menu has been implemented to display events in descending order of id

[jdamore-linode]

Yeah, this stems from when we had briefly been sorting by created, if I remember correctly, and one of the implications was that we could no longer assume the event at index 0 had the highest ID and therefore had to account for that when marking events as seen.

We undid that change, but since there was still a lot of uncertainty around where we would land, I just skipped the test and didn't remove it entirely. It's safe to get rid of it now, I'd say.

[hkhalil-akamai]

A couple of the tests in file are being skipped currently -- intentional?

[jdamore-linode]

Yes, they're failing in CI. I probably won't have time to fix them before we merge this but want to fix them eventually.

[bnussman-akamai]

I'm wondering if this needs to be updated

manager/packages/manager/src/features/Images/ImagesLanding/ImagesLanding.tsx

Lines 195 to 201 in 8176f7a

	const { events } = useEventsInfiniteQuery();
	const imageEvents =
	events?.filter(
	(event) =>
	isEventInProgressDiskImagize(event) || isEventImageUpload(event)
	) ?? [];

I don't have lots of context ImagesLanding's use of events

[abailly-akamai]

closing as rebase is difficult. moved to #10899