Data streams improvements by axw · Pull Request #4449 · elastic/apm-server

axw · 2020-11-20T09:51:50Z

Motivation/summary

Separate data streams support from existing index management and selector, and extend config validation to prevent mixing data streams and existing index management config (ILM, pipelines, template setup, index names, etc.)

When the server is running under Agent, it is now necessary for data streams to be enabled. We should inject the config (-E apm-server.data_streams.enabled) from the program spec.

Finally, we introduce system tests to show that with data streams enabled, the server works when given only limited privileges like those given by Fleet.

Checklist

I have signed the Contributor License Agreement.
~~- [ ] I have updated CHANGELOG.asciidoc~~

I have considered changes for:
~~- [ ] documentation~~ (later)

logging (add log lines, choose appropriate log selector, etc.)
~~- [ ] metrics and monitoring (create issue for Kibana team to add metrics to visualizations, e.g. Kibana#44001)~~
automated tests (add tests for the code changes, all unit tests pass locally)
~~- [ ] telemetry~~
~~- [ ] Elasticsearch Service (https://cloud.elastic.co)~~
~~- [ ] Elastic Cloud Enterprise (https://www.elastic.co/products/ece)~~
~~- [ ] Elastic Cloud on Kubernetes (https://www.elastic.co/elastic-cloud-kubernetes)~~

How to test these changes

Run the server with apm-server.data_streams.enabled=true, should index into traces-* etc., with no pipeline specified at index time.
Run apm-server setup with apm-server.data_streams.enabled=true, should fail.
Run apm-server setup --pipelines with apm-server.data_streams.enabled=true, should fail.

Related issues

#4378

Create a completely separate Supporter when data streams are enabled. If data streams are enabled, ensure that no setup (template, ILM, pipeline) related config is specified.

ghost · 2020-11-20T09:55:32Z

💚 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

Build Cause: axw commented: jenkins run the tests please
Start Time: 2020-12-01T06:23:16.413+0000
Duration: 41 min 56 sec

Test stats 🧪

Test	Results
Failed	0
Passed	4623
Skipped	141
Total	4764

Steps errors

Expand to view the steps failures

`Compress`

Took 0 min 0 sec . View more details on here
Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage

`Compress`

Took 0 min 0 sec . View more details on here
Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests

`Test Sync`

Took 3 min 49 sec . View more details on here
Description: ./.ci/scripts/sync.sh

codecov-io · 2020-11-20T10:02:31Z

Codecov Report

Merging #4449 (f9dea92) into master (b0e6d02) will increase coverage by 0.02%.
The diff coverage is 70.90%.

@@            Coverage Diff             @@
##           master    #4449      +/-   ##
==========================================
+ Coverage   76.04%   76.07%   +0.02%     
==========================================
  Files         159      160       +1     
  Lines        9748     9775      +27     
==========================================
+ Hits         7413     7436      +23     
- Misses       2335     2339       +4

Impacted Files	Coverage Δ
beater/beater.go	`58.20% <36.84%> (-3.45%)`	⬇️
idxmgmt/datastreams.go	`63.63% <63.63%> (ø)`
beater/config/config.go	`68.33% <100.00%> (+3.33%)`	⬆️
idxmgmt/supporter.go	`85.22% <100.00%> (+7.67%)`	⬆️
idxmgmt/supporter_factory.go	`83.33% <100.00%> (+7.40%)`	⬆️
processor/otel/consumer.go	`93.51% <0.00%> (-0.45%)`	⬇️

jalvz · 2020-11-20T13:46:35Z

When the server is running under Agent, it is now necessary for data streams to be enabled

elastic/beats#22689

If data streams are enabled, pipeline registration is disabled and attempting to setup pipelines will fail. No pipeline will be specified when indexing; the pipeline must be specified in the index template. Also, if the server is running managed (by Fleet), the server will fail to start unless data streams are enabled.

simitt

apm-server.ilm.* configurations (including apm-server.ilm.setup.enabled=false and
apm-server.ilm.enabled=false) are not allowed when using datastreams - but configuring setup.template.* is allowed and ignored.
From a migration perspective it might be easier for users if their configurations for ILM and templates were still allowed but ignored (and a warning is logged).

Apart from that - changes look good and on a quick test run work as expected.

systemtest/elasticsearch.go

axw · 2020-11-23T09:28:34Z

apm-server.ilm.* configurations (including apm-server.ilm.setup.enabled=false and
apm-server.ilm.enabled=false) are not allowed when using datastreams - but configuring setup.template.* is allowed and ignored.

Oops, that was an oversight.

From a migration perspective it might be easier for users if their configurations for ILM and templates were still allowed but ignored (and a warning is logged).

That's a good point, I suppose it's a bit friendlier to log warnings instead. I'll update to do that.

If legacy config is defined along with data streams config, log warnings rather than returning errors. Remove libbeat template setup config overrides and move the overrides into idxmgmt. This is necessary to detect when setup.template is explicitly specified by the user, vs. injected by our code.

oops

axw · 2020-11-24T07:19:03Z

I broke things by moving the ConditionalOverride stuff, looking into it now.

simitt

I believe the two failing system tests actually uncover a bug in the current behavior - the APM Server injected changes should not show up as config changes, but they should be part of the exported template (as they are now) - so changing the expectations to fix the tests would be fine.

idxmgmt/supporter_factory.go

Perform some gymnastics in order to only set legacy defaults when data streams are not enabled. Migrate "export config" system tests to Go.

jalvz · 2020-11-24T10:28:46Z

should we update docs/data/elasticsearch/generated with the new dataset fields?

axw · 2020-11-24T10:31:01Z

I believe the two failing system tests actually uncover a bug in the current behavior - the APM Server injected changes should not show up as config changes, but they should be part of the exported template (as they are now) - so changing the expectations to fix the tests would be fine.

Yes indeed, you make a good point. I should have thought about that a bit harder. I managed to get it to work with config overrides, but it would be better if these defaults don't show up. I'm not sure how we can stop the logging overrides from showing up there. I'll dig a bit more...

axw · 2020-11-24T10:31:45Z

should we update docs/data/elasticsearch/generated with the new dataset fields?

@jalvz IMO we should hold off for now. The initial release will be very experimental, so probably better not to advertise so widely.

axw · 2020-11-24T10:48:23Z

Yes indeed, you make a good point. I should have thought about that a bit harder. I managed to get it to work with config overrides, but it would be better if these defaults don't show up. I'm not sure how we can stop the logging overrides from showing up there. I'll dig a bit more...

I'm going to pull this out into a separate PR.

beater/beater.go

cmd/root.go

idxmgmt/datastreams.go

axw · 2020-12-01T06:22:49Z

jenkins run the tests please

* idxmgmt: create independent data streams Supporter Create a completely separate Supporter when data streams are enabled. If data streams are enabled, ensure that no setup (template, ILM, pipeline) related config is specified. * beater: enabling data streams disables pipeline If data streams are enabled, pipeline registration is disabled and attempting to setup pipelines will fail. No pipeline will be specified when indexing; the pipeline must be specified in the index template. Also, if the server is running managed (by Fleet), the server will fail to start unless data streams are enabled. * systemtest: add tests for data streams * idxmgmt: log warnings upon finding legacy config If legacy config is defined along with data streams config, log warnings rather than returning errors.

simitt · 2020-12-23T20:28:09Z

Tested with BC1

export, setup behave as described
no pipelines are setup
if apm-server.data_streams.enabled: true and apm-server.ilm.setup: true a message is logged:

{"log.level":"warn","@timestamp":"2020-12-23T21:26:42.393+0100","log.logger":"index-management","log.origin":{"file.name":"idxmgmt/supporter_factory.go","file.line":146},"message":"`apm-server.ilm` specified, but will be ignored as data streams are enabled","ecs.version":"1.6.0"}

idxmgmt: create independent data streams Supporter

a0c578c

Create a completely separate Supporter when data streams are enabled. If data streams are enabled, ensure that no setup (template, ILM, pipeline) related config is specified.

axw force-pushed the datastreams-supporter branch from 380e936 to 1bb9da4 Compare November 20, 2020 09:52

jalvz mentioned this pull request Nov 20, 2020

Enable data streams for apm-server when running under Agent elastic/beats#22689

Merged

6 tasks

axw added 2 commits November 21, 2020 08:43

systemtest: add tests for data streams

dd14171

axw force-pushed the datastreams-supporter branch from 1bb9da4 to dd14171 Compare November 21, 2020 00:44

axw marked this pull request as ready for review November 21, 2020 01:54

axw requested a review from a team November 21, 2020 01:54

jalvz mentioned this pull request Nov 23, 2020

Integrate with Elastic Agent #4004

Closed

15 tasks

simitt reviewed Nov 23, 2020

View reviewed changes

systemtest/elasticsearch.go Show resolved Hide resolved

systemtest: delete logs-apm.*

9a07f35

axw added 3 commits November 24, 2020 11:36

Merge branch 'master' into datastreams-supporter

d6e71d8

idxmgmt: remove empty if statement

976bea9

oops

simitt reviewed Nov 24, 2020

View reviewed changes

idxmgmt/supporter_factory.go Outdated Show resolved Hide resolved

Move default config overrides back to cmd

2b0c563

Perform some gymnastics in order to only set legacy defaults when data streams are not enabled. Migrate "export config" system tests to Go.

WIP

3077271

axw mentioned this pull request Nov 25, 2020

Move APM-specific template defaults to idxmgmt #4458

Merged

3 tasks

jalvz approved these changes Nov 25, 2020

View reviewed changes

beater/beater.go Outdated Show resolved Hide resolved

cmd/root.go Outdated Show resolved Hide resolved

idxmgmt/datastreams.go Outdated Show resolved Hide resolved

axw added 2 commits November 30, 2020 17:01

Merge branch 'master' into datastreams-supporter

84d0d47

Merge branch 'master' into datastreams-supporter

337353e

axw added 2 commits December 1, 2020 11:47

Mention apm integration package in errors

ae7f8a5

Update system tests

f9dea92

axw merged commit 49d55cc into elastic:master Dec 1, 2020

axw deleted the datastreams-supporter branch December 1, 2020 07:12

axw mentioned this pull request Dec 14, 2020

[7.x] Data streams improvements (#4449) #4529

Merged

simitt added test-plan v7.11.0 labels Dec 16, 2020

simitt added the test-plan-ok label Dec 23, 2020

simitt self-assigned this Dec 23, 2020

Conversation

axw commented Nov 20, 2020

Motivation/summary

Checklist

How to test these changes

Related issues

Uh oh!

ghost commented Nov 20, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Test stats 🧪

Steps errors

Compress

Compress

Test Sync

Uh oh!

codecov-io commented Nov 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jalvz commented Nov 20, 2020

Uh oh!

simitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

axw commented Nov 23, 2020

Uh oh!

axw commented Nov 24, 2020

Uh oh!

simitt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jalvz commented Nov 24, 2020

Uh oh!

axw commented Nov 24, 2020

Uh oh!

axw commented Nov 24, 2020

Uh oh!

axw commented Nov 24, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

axw commented Dec 1, 2020

Uh oh!

simitt commented Dec 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ghost commented Nov 20, 2020 •

edited by ghost

Loading

`Compress`

`Compress`

`Test Sync`

codecov-io commented Nov 20, 2020 •

edited

Loading