sql: include sampled stats in TestSampledStatsCollection

[asubiotto]

Depends on #59992, which is required for this new regression test to pass.

TestSampledStatsCollection would previously only check that stats that are
collected regardless of the sample rate are returned. These types of stats
(rows/bytes read) are propagated using metadata, rather than the trace.

This resulted in us silently failing to collect any stats when sampling was
enabled once the tracing mode was reverted back to legacy. To avoid this kind
of thing happening again, this commit adds a check that max memory usage is
reported to be non-zero.

Release note: None (this is a new feature that has no user impact yet)

[cockroach-teamcity]

This change is 

[yuzefovich]

The last commit

Reviewed 2 of 2 files at r5.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @asubiotto)

---

pkg/sql/instrumentation_test.go, line 83 at r5 (raw file):

		require.Equal(t, int64(1), stats.mu.data.ExecStatCollectionCount, "expected to have collected exactly one set of execution stats")
		require.Greater(t, stats.mu.data.RowsRead.Mean, float64(0), "expected statement to have read at least one row")
		require.Greater(t, stats.mu.data.MaxMemUsage.Mean, float64(0), "expected statement to have requested bytes")

nit: to me "to have requested bytes" reads like it is more about BytesRead and not MaxMemUsage, maybe expected statement to request some RAM or something like that?

[asubiotto]

Reviewable status: complete! 1 of 0 LGTMs obtained

---

pkg/sql/instrumentation_test.go, line 83 at r5 (raw file):

Previously, yuzefovich (Yahor Yuzefovich) wrote…

nit: to me "to have requested bytes" reads like it is more about BytesRead and not MaxMemUsage, maybe expected statement to request some RAM or something like that?

Done.

[asubiotto]

TFTR

bors r=yuzefovich

[craig]

Build succeeded:

• GitHub CI (Cockroach)