Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

modelindexer: introduce go-elasticsearch indexer #5970

Merged
merged 12 commits into from
Oct 11, 2021

Conversation

axw
Copy link
Member

@axw axw commented Aug 19, 2021

Motivation/summary

Introduce an experimental option to index events using go-elasticsearch, bypassing libbeat. This only works when data streams are enabled. You can enable the experimental indexer with:

apm-server -E output.elasticsearch.experimental=true -E apm-server.data_streams.enabled=true

This approach aligns with how we want to evolve apm-server output tuning: moving away from the current libbeat queue and output configuration, simplifying config to be similar to how APM Agents are configured: bulk requests are now executed either after some time elapses (output.elasticsearch.flush_interval, defaults to 1s) or if a number of bytes is reached (output.elasticsearch.flush_bytes, defaults to 5MB).

We have temporarily forked the go-elasticsearch bulk-indexer. See comments in the code for rationale.

Some libbeat metrics are faked, so this output works with stack monitoring and other existing uses of libbeat metrics. We should consider removing "libbeat" from our metric names and standardising on those, so we are not locked into a particular technology.

Checklist

How to test these changes

  1. Run apm-server with -E output.elasticsearch.experimental=true -E apm-server.data_streams.enabled=true
  2. Send various events, check they are indexed successfully

Related issues

#6002

@apmmachine
Copy link
Contributor

apmmachine commented Aug 19, 2021

❕ Build Aborted

There is a new build on-going so the previous on-going builds have been aborted.

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Reason: Aborted from #19

  • Start Time: 2021-10-11T07:59:42.389+0000

  • Duration: 33 min 27 sec

  • Commit: 5084eba

Test stats 🧪

Test Results
Failed 0
Passed 5688
Skipped 17
Total 5705

Steps errors 1

Expand to view the steps failures

Print Message
  • Took 0 min 0 sec . View more details here
  • Description: �[39;49m[INFO] Can not determine redirect link!!!�[0m

🤖 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.

@axw axw force-pushed the publish-goelasticsearch branch from be2c558 to 2ca39f7 Compare August 23, 2021 09:29
@axw axw force-pushed the publish-goelasticsearch branch 2 times, most recently from 8943471 to 0aeebb2 Compare September 3, 2021 07:24
@axw axw force-pushed the publish-goelasticsearch branch 4 times, most recently from a00c818 to 9ac9844 Compare September 15, 2021 10:02
@axw axw force-pushed the publish-goelasticsearch branch 2 times, most recently from 266196b to 0320bae Compare October 7, 2021 04:51
@axw
Copy link
Member Author

axw commented Oct 7, 2021

publish (libbeat):

go test -benchmem -bench=. -run=NONE -benchtime=5s -cpu=1,6,12
goos: linux
goarch: amd64
pkg: github.com/elastic/apm-server/publish
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkPublisher                329248             17501 ns/op            1931 B/op         19 allocs/op
BenchmarkPublisher-6              463827             11226 ns/op            1935 B/op         19 allocs/op
BenchmarkPublisher-12             360158             14351 ns/op            1935 B/op         19 allocs/op
PASS
ok      github.com/elastic/apm-server/publish   54.398s

modelindexer:

$ go test -v -benchmem -bench=. -benchtime=5s -run=NONE -cpu=1,6,12
goos: linux
goarch: amd64
pkg: github.com/elastic/apm-server/model/modelindexer
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkModelIndexer
BenchmarkModelIndexer            1253119              5272 ns/op            1303 B/op         12 allocs/op
BenchmarkModelIndexer-6          2137680              2537 ns/op            1241 B/op         12 allocs/op
BenchmarkModelIndexer-12         1768158              2898 ns/op            1259 B/op         12 allocs/op
PASS
ok      github.com/elastic/apm-server/model/modelindexer        25.672s

@axw axw force-pushed the publish-goelasticsearch branch 2 times, most recently from f2a07a3 to 66b59df Compare October 7, 2021 06:06
Introduce an experimental option to index events
using go-elasticsearch, bypassing libbeat.
@axw axw force-pushed the publish-goelasticsearch branch from 66b59df to 6f7fb2b Compare October 7, 2021 06:13
@axw axw marked this pull request as ready for review October 7, 2021 07:10
@axw axw requested a review from a team October 7, 2021 07:10
Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking really good!

model/modelindexer/bulk_indexer.go Outdated Show resolved Hide resolved
model/modelindexer/indexer.go Outdated Show resolved Hide resolved
model/modelindexer/indexer.go Outdated Show resolved Hide resolved
model/modelindexer/indexer.go Show resolved Hide resolved
Copy link
Contributor

@marclop marclop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

beater/beater.go Outdated Show resolved Hide resolved
beater/server_test.go Show resolved Hide resolved
model/modelindexer/bulk_indexer.go Outdated Show resolved Hide resolved
model/modelindexer/indexer.go Show resolved Hide resolved
model/modelindexer/indexer_integration_test.go Outdated Show resolved Hide resolved
axw added 4 commits October 11, 2021 13:12
Don't let flushing block Close. Add a context param
to Close; when it is cancelled, cancel any ongoing
flush attempts.
Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This loooks great!

Tested a bit with standalone+data streams and also when running managed by agents, didn't observe any obvious issues.

beater/beater.go Outdated Show resolved Hide resolved
var eventsFailed int64
for _, item := range resp.Items {
for _, info := range item {
if info.Error.Type != "" || info.Status > 201 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not certain we need this, but libbeat did check for status < 500 per event, before calling an event nonIndexable and dropping it.

Maybe for later, libbeat just recently introduced sending events that can't be indexed to a dead letter index (if configured).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think I'll defer these kinds of things, I'd like to get this merged so we can test out the basics first.

@apmmachine
Copy link
Contributor

apmmachine commented Oct 11, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2021-10-11T10:34:24.206+0000

  • Duration: 44 min 55 sec

  • Commit: a3003da

Test stats 🧪

Test Results
Failed 0
Passed 6185
Skipped 18
Total 6203

🤖 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.

@axw axw enabled auto-merge (squash) October 11, 2021 10:33
@axw axw merged commit aa4f76a into elastic:master Oct 11, 2021
mergify bot pushed a commit that referenced this pull request Oct 11, 2021
* modelindexer: introduce go-elasticsearch indexer

Introduce an experimental option to index events
using go-elasticsearch, bypassing libbeat.

(cherry picked from commit aa4f76a)

# Conflicts:
#	changelogs/head.asciidoc
axw added a commit that referenced this pull request Oct 12, 2021
… (#6317)

* modelindexer: introduce go-elasticsearch indexer (#5970)

* modelindexer: introduce go-elasticsearch indexer

Introduce an experimental option to index events
using go-elasticsearch, bypassing libbeat.

(cherry picked from commit aa4f76a)

# Conflicts:
#	changelogs/head.asciidoc

* Delete head.asciidoc

Co-authored-by: Andrew Wilkins <[email protected]>
@marclop marclop added backport-skip Skip notification from the automated backport with mergify test-plan labels Oct 25, 2021
@stuartnelson3 stuartnelson3 self-assigned this Nov 4, 2021
@stuartnelson3
Copy link
Contributor

Confirmed with BC3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify test-plan test-plan-ok v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants