This pull request does not have a backport label. Could you fix it @lahsivjar? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-7.x is the label to automatically backport to the 7.x branch.
• backport-7./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /hey-apm : Run the hey-apm benchmark.

• /package : Generate and publish the docker images.

• /test windows : Build & tests on Windows.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

🌐 Coverage report

@axw I agree with the output getting unwieldy. For automated benchmark, IMO, we should always index the results to ES but for other use cases around manual benchmarking I think optional metric opt-in flag for extra metrics will be really useful. @marclop WDYT?

@axw I think that would be better and we should consider categorizing the metrics in essential and optional categories. This may improve the output and the mental strain of parsing all these number thrown at you. I should've added some of these ideas to the the original issue when I created it and start the discussion then.

There are some metrics that aren't really that useful, particularly when the number is 0, one that comes to mind is error_responses/sec.

Since we mimick the output of go test -bench, we have certain constraints on what we need to publish (name, iterations and ns/op), additionally we print B/op and allocs/op which would be the metrics reported when -benchmem is set in go test -bench.

Of our custom metrics, we don't really need all of them by default, if we're just doing some quick benchmarks, it may be better to limit the number of other metrics that are printed out. I believe that it would be more effective to print less metrics by default and omit those that are 0. the output of these could be as below (the final number of metrics to be printed is yet TBD)

$ ./apmbench
benchmarkName   100   1000 ns/op  6000 events/sec   1276948 B/op   2831 allocs/op

In case that there are some error responses, we could report those, but they needn't be included in all the results:

$ ./apmbench
benchmarkName   100   1000 ns/op   1 error_responses/sec  6000 events/sec   1276948 B/op   2831 allocs/op

We can introduce a new flag that when set, includes all the metrics that we're recording to be displayed -detailed or a similar flag name could work:

$ ./apmbench -detailed
benchmarkName   100   1000 ns/op   1 error_responses/sec  6000 events/sec   1000 errors/sec 1000 metrics/sec   1000 txs/sec   3000 spans/sec   10 gc/op   1276948 B/op   2831 allocs/op ...

I think that the usefulness of the broken down event/s (metrics, errors, txs, spans) metrics is not immediately obvious and we should most likely omit that to free some space.

Thanks for the proposal @marclop, that all sounds good to me.

@marclop What is your recommendation on the extra metrics for rss, goroutines, indexers, gc and heap? I see in the table that you have only kept gc cycles and max rss. Is that all we want to keep?

Also, regarding max_rss, I am not sure if that will be enough to derive a lot of data from. IIUC, max_rss will also include the memory not released to OS and it might be possible that max_rss remains same throughout the benchmark process especially if the same apm-server process is used in multiple benchmarks.

This pull request is now in conflicts. Could you fix it @lahsivjar? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 7817_usage_metrics upstream/7817_usage_metrics
git merge upstream/main
git push upstream 7817_usage_metrics

/test