Add assertions to OpenTelemetryNodePagePerformanceTest::testNodePageCoolCache()

Actually curious why validation went down? Hope not a trend of a new random

Fail is not relevant. Let's relaunch it

There was 1 failure:
1)
Drupal\Tests\image\Functional\ImageStylesPathAndUrlTest::testImageStylePrivateWithConversion
Behat\Mink\Exception\ExpectationException: Current response status code is
403, but 200 expected.

The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

False positive

That was 1 heck of a query. Looking at the query numbers at the end those and numbers i believe seem right. Least not crazy ones.

This looks good to me. That is indeed a lot of database queries.

Committed/pushed to 11.x and cherry-picked to 11.1.x, thanks!

This just failed randomly https://git.drupalcode.org/project/drupal/-/jobs/3861317

Reverting for now.

The query list seems like it might be quite painful to maintain and will get rather dynamic. I'd assume that fibers suspend and resume isn't happening in a reliably way. With 📌 Try to replace path alias preloading with lazy generation Active we'd end up with a variable amount of grouped path alias lookup queries for example.

Another more simple example are node ids. I don't know how umami creates its default content right now, but the recipe default content doesn't have stable Id's, so we'd need to make all those ids and revision ids.

Might be better to assert those things more selectively and pick out things that we expect to be more stable. For example, we group out all queries against the config table and assert them specifically?

I'd assume that fibers suspend and resume isn't happening in a reliable way.

This isn't true yet, the page execution will happen reliably in the same order assuming an identical starting point, but it will become true once we add ✨ [PP-1] Async database query + fiber support Active and start using it.

However we already have the problem (for performance testing) that asset aggregates and the main request can finish in any order, and that affects cache collector due to the way it cumulatively caches end of request, I think that's what we saw with the test failure in HEAD.

Node IDs and page content are currently stable since 🐛 Umami content is all created in the same second Active , that was an early problem introducing performance test coverage. We can add placeholders for NODE_ID and similar to make the assertions more agnostic. It's going to be a general problem with default content though.

Similar discussion is happening in 📌 Switch the performance job to fail Active , where I got to there was using array_intersect() on the expected and actual queries so that new ones can't be added and the order doesn't matter.

It would definitely be good to be able to add the coverage, just based on the issues you already opened from this one it shows we have plenty of things we can fix here, and a way to measure it, but it's definitely hard work.