Add gencorpora cmd to generate ES corpora for rally #8878

lahsivjar · 2022-08-16T15:11:43Z

Motivation/summary

Add command for generating ES corpus based on APM agent data sent to a test APM-Server.

Checklist

~~- [ ] Update CHANGELOG.asciidoc~~
~~- [ ] Update package changelog.yml (only if changes to apmpackage have been made)~~
~~- [ ] Documentation has been updated~~

How to test these changes

$ cd systemtest/cmd/gencorpora
$ go run main.go

Examine stdout for ES corpus.

Related issues

Part of #8754

mergify · 2022-08-16T15:12:37Z

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.

mergify · 2022-08-16T15:12:38Z

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.

apmmachine · 2022-08-16T15:19:12Z

💚 Build Succeeded

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

Expand to view the summary

Build stats

Start Time: 2022-08-18T09:33:05.530+0000
Duration: 24 min 46 sec

Test stats 🧪

Test	Results
Failed	0
Passed	128
Skipped	1
Total	129

🤖 GitHub comments

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

/test : Re-trigger the build.
/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!)

marclop

couple of small comments.

systemtest/cmd/gencorpora/main.go

systemtest/gencorpora/catbulk.go

apmmachine · 2022-08-17T07:34:17Z

📚 Go benchmark report

Diff with the main branch

name                                                            old time/op    new time/op    delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64
ShardedWriteTransactionUncontended-12                              844ns ± 7%    1001ns ±23%  +18.65%  (p=0.032 n=5+5)
ReadEvents/json_codec_big_tx/1_events-12                          10.9µs ± 6%    11.7µs ± 4%   +7.23%  (p=0.008 n=5+5)
ReadEvents/nop_codec/10_events-12                                 21.1µs ±32%    15.1µs ±28%  -28.42%  (p=0.032 n=5+5)

name                                                            old alloc/op   new alloc/op   delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
Process-12                                                        10.0kB ± 2%     9.6kB ± 0%   -3.56%  (p=0.016 n=5+4)
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64
WriteTransaction/json_codec_big_tx-12                             3.52kB ± 0%    3.52kB ± 0%   +0.03%  (p=0.016 n=5+4)

name                                                            old allocs/op  new allocs/op  delta
pkg:github.com/elastic/apm-server/internal/agentcfg goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/decoder goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/model/modelindexer goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/processor/stream goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/internal/publish goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/spanmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/aggregation/txmetrics goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling goos:linux goarch:amd64
pkg:github.com/elastic/apm-server/x-pack/apm-server/sampling/eventstorage goos:linux goarch:amd64

report generated with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat

lahsivjar · 2022-08-17T10:31:34Z

/test

axw

Only scanned over so far, but what I've seen looks good - I will be happy to see cat_bulk.py replaced. Main question so far is whether we can use apmservertest.Server.

axw · 2022-08-18T05:14:00Z

systemtest/gencorpora/apmserver.go

+// APMServer represents a wrapped exec CMD for APMServer process
+type APMServer struct {
+	apmHost string
+	cmd     *apmservertest.ServerCmd


Is there a reason why you're not using apmservertest.Server?

I read a little further and probably answered my own question: apmservertest.Server requires a testing.TB. Maybe we can get rid of that requirement?

That was my first approach but I decided to abandon it because the apmservertest.Server is tightly coupled with testing.TB utilizing Cleanup function to prevent unnecessary handling to leak into the test cases. I was thinking to extend the APMServer command to meet the requirements of test cases in a generic way and then replace apmservertest#Server to use it with testing.TB. But, this proved to be a lot more work than I anticipated and I thought we can address it at a later point. Let me know if you think it should be addressed now.

Yes, that's fair enough.

I spiked on making the testing.TB part optional - I might create a followup on your PR if you don't mind.

lahsivjar · 2022-08-18T05:54:06Z

I will be happy to see cat_bulk.py replaced

@axw Yup, I am working on this in my next PR.

axw

LGTM, just a couple of nitpicks

systemtest/gencorpora/catbulk.go

lahsivjar force-pushed the 8754_rally branch from a559652 to 496dcec Compare August 16, 2022 15:12

mergify bot added the backport-skip Skip notification from the automated backport with mergify label Aug 16, 2022

lahsivjar force-pushed the 8754_rally branch from 496dcec to b6a8aac Compare August 16, 2022 15:31

marclop reviewed Aug 17, 2022

View reviewed changes

systemtest/cmd/gencorpora/main.go Outdated Show resolved Hide resolved

systemtest/gencorpora/catbulk.go Outdated Show resolved Hide resolved

systemtest/gencorpora/catbulk.go Show resolved Hide resolved

lahsivjar force-pushed the 8754_rally branch 5 times, most recently from a93aba6 to 6b0cdf6 Compare August 17, 2022 07:04

lahsivjar marked this pull request as ready for review August 17, 2022 07:05

lahsivjar requested review from a team and marclop August 17, 2022 07:05

Add gencorpora cmd to generate ES corpora for rally

cbaf7df

lahsivjar force-pushed the 8754_rally branch from 6b0cdf6 to cbaf7df Compare August 17, 2022 07:08

lahsivjar added 2 commits August 17, 2022 15:47

Minor formatting

581f919

Handle interrupts

cea4ae2

Fix interrupt process

d9c672b

axw reviewed Aug 18, 2022

View reviewed changes

axw approved these changes Aug 18, 2022

View reviewed changes

systemtest/gencorpora/catbulk.go Outdated Show resolved Hide resolved

systemtest/gencorpora/catbulk.go Outdated Show resolved Hide resolved

lahsivjar added 2 commits August 18, 2022 17:32

Address review comments

1e252f2

Merge branch 'main' into 8754_rally

1db3157

lahsivjar enabled auto-merge (squash) August 18, 2022 09:36

lahsivjar merged commit 8f4f7b0 into elastic:main Aug 18, 2022

lahsivjar deleted the 8754_rally branch August 18, 2022 10:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gencorpora cmd to generate ES corpora for rally #8878

Add gencorpora cmd to generate ES corpora for rally #8878

lahsivjar commented Aug 16, 2022

mergify bot commented Aug 16, 2022

mergify bot commented Aug 16, 2022

apmmachine commented Aug 16, 2022 •

edited

Loading

Build stats

Test stats 🧪

marclop left a comment

apmmachine commented Aug 17, 2022 •

edited

Loading

lahsivjar commented Aug 17, 2022

axw left a comment

axw Aug 18, 2022

axw Aug 18, 2022

lahsivjar Aug 18, 2022

axw Aug 18, 2022

lahsivjar commented Aug 18, 2022

axw left a comment

Add gencorpora cmd to generate ES corpora for rally #8878

Add gencorpora cmd to generate ES corpora for rally #8878

Conversation

lahsivjar commented Aug 16, 2022

Motivation/summary

Checklist

How to test these changes

Related issues

mergify bot commented Aug 16, 2022

mergify bot commented Aug 16, 2022

apmmachine commented Aug 16, 2022 • edited Loading

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

marclop left a comment

Choose a reason for hiding this comment

apmmachine commented Aug 17, 2022 • edited Loading

📚 Go benchmark report

lahsivjar commented Aug 17, 2022

axw left a comment

Choose a reason for hiding this comment

axw Aug 18, 2022

Choose a reason for hiding this comment

axw Aug 18, 2022

Choose a reason for hiding this comment

lahsivjar Aug 18, 2022

Choose a reason for hiding this comment

axw Aug 18, 2022

Choose a reason for hiding this comment

lahsivjar commented Aug 18, 2022

axw left a comment

Choose a reason for hiding this comment

apmmachine commented Aug 16, 2022 •

edited

Loading

apmmachine commented Aug 17, 2022 •

edited

Loading