Use _doc if ES major version is 7 by urso · Pull Request #9056 · elastic/beats

urso · 2018-11-13T11:41:22Z

Update the Elasticsearch output and template generator to set the type
to _doc if Elasticsearch major version is 7.

libbeat/outputs/elasticsearch/client.go

ruflin

This needs a changelog entry.

libbeat/outputs/elasticsearch/client.go

libbeat/template/template.go

libbeat/outputs/elasticsearch/client.go

libbeat/kibana/client.go

libbeat/common/version.go

ruflin

Left a few minor comments. Code LGTM. Could you make hound happy?

CHANGELOG.asciidoc

filebeat/beater/filebeat.go

ruflin · 2018-11-19T08:37:05Z

libbeat/common/version.go

Should this be called IsEmpty instead?

Don't care about the name. stdlib seems to IsZero() bool for some types.

Keeping IsValid cause of docstring:

IsValid returns true if the version object stores a successfully parsed version number.

The NewVersion constructor is no simple constructor, but actually a parser. Wonder why we parse ourselves. github.com/hashicorp/go-version is a pretty good and flexible versions package:

ruflin · 2018-11-19T08:37:45Z

libbeat/dashboards/dashboards.go

Not related to this PR but I wonder if we should remove support for this.

@jsoriano More related to your other PR with removing 5.x dashboards.

We can consider to keep this in libbeat in case some community beat still wants to try to keep support for 5.x.

If not, I can remove it in in #8927, along with the importViaES method.

I would rather remove it to reduce complexity in the code. Can also be done in a follow up PR to keep things smaller.

ruflin · 2018-11-19T08:41:08Z

libbeat/template/template.go

I wonder if we should even allow to run Beats 7.0 against ES 5.x?

@monicasarbu @tsg

urso · 2018-11-20T12:39:40Z

@ruflin This PR will be for 7.0 and 6.7 the least. I'd say we should remove ES 5.x support in 7.0 in a followup PR (will add item to 7.0 bc meta issue).

ruflin · 2018-11-23T06:39:32Z

@urso Are the failing tests related to the flaky ones?

urso · 2018-11-23T13:51:07Z

Some tests are still failing. Some code seems in to sneak in a type named doc, so we end up with 2 types _doc and doc. Still trying to find the root cause.

Update the Elasticsearch output and template generator to set the type to _doc if Elasticsearch major version is 7.

- Use common version instead of strings + parsing over and over again. - Having the parsed version available earlier, we can now configure the default document type based on version ranges. - Use `_doc` if version is >= 7.0.

ruflin

Only issue is changelog nit.

CHANGELOG.asciidoc

ruflin · 2018-11-29T09:06:32Z

filebeat/fileset/fileset_test.go

-	marshaled, err = json.Marshal(content)
-	assert.NoError(t, err)
-	assert.Contains(t, string(marshaled), "beat.timezone")
+	cases := map[string]struct {


Much nicer 👍

ruflin · 2018-11-29T09:09:21Z

libbeat/outputs/elasticsearch/api_integration_test.go

 	client := getTestingElasticsearch(t)
-	if strings.HasPrefix(client.Connection.version, "2.") {
-		t.Skip("Skipping tests as pipeline not available in 2.x releases")
+	if client.Connection.version.Major < 5 {


Hopefully we can clean up this code one day as we should not test anymore against 2.x I think in the future.

Update the Elasticsearch output and template generator to set the type to _doc if Elasticsearch major version is 7. Use common.Version throughout - Use common version instead of strings + parsing over and over again. - Having the parsed version available earlier, we can now configure the default document type based on version ranges. - Use `_doc` if version is >= 7.0. Introduce unit tests for version aware bulk encoding. (cherry picked from commit 41f87a4)

…rsion (#9813) Partial backport of #9056 — only the code changes related to changing the datatype of version fields/variables from `string` to `common.Version`.

…rsion (#9813) (#9815) Partial backport of #9056 — only the code changes related to changing the datatype of version fields/variables from `string` to `common.Version`. (cherry picked from commit c23a0b4)

…rsion (elastic#9813) (elastic#9815) Partial backport of elastic#9056 — only the code changes related to changing the datatype of version fields/variables from `string` to `common.Version`. (cherry picked from commit dafb638)

urso added the review label Nov 13, 2018

urso requested review from ruflin and tsg November 13, 2018 11:41

houndci-bot reviewed Nov 13, 2018

View reviewed changes

libbeat/outputs/elasticsearch/client.go Outdated Show resolved Hide resolved

ruflin requested changes Nov 14, 2018

View reviewed changes

libbeat/outputs/elasticsearch/client.go Outdated Show resolved Hide resolved

libbeat/template/template.go Outdated Show resolved Hide resolved

houndci-bot reviewed Nov 15, 2018

View reviewed changes

libbeat/outputs/elasticsearch/client.go Outdated Show resolved Hide resolved

libbeat/kibana/client.go Outdated Show resolved Hide resolved

libbeat/common/version.go Outdated Show resolved Hide resolved

urso force-pushed the doc-type-for-7 branch from c7126d0 to c451beb Compare November 15, 2018 21:43

ruflin reviewed Nov 19, 2018

View reviewed changes

urso force-pushed the doc-type-for-7 branch from c3a2891 to ae17426 Compare November 22, 2018 22:34

ph added the libbeat label Nov 23, 2018

urso added 17 commits November 27, 2018 23:23

Use _doc if ES major version is 7

4dab935

Update the Elasticsearch output and template generator to set the type to _doc if Elasticsearch major version is 7.

Use common.Version throughout

fddc6cb

- Use common version instead of strings + parsing over and over again. - Having the parsed version available earlier, we can now configure the default document type based on version ranges. - Use `_doc` if version is >= 7.0.

cleanup

19309fa

Added changelog

7c6d01b

Fix template unit tests

581f1e6

Review updates

9072cd4

Add docstring to es Connect methods

3a0a6cb

Add MustNewVersion

2e80753

Fix wrong condition on version

c0465b6

Update unit tests

c393fb3

Version check in fileset integration test

45a17fe

Add unit tests for ES bulk encoder

e402d62

Use ES version in template integration test

a1b43b2

Fix dashboard loaders

3b3e7bd

Use _doc in tester

58202ca

Use _doc when indexing via LS

bae824d

Fix console output test

fffd320

urso added 6 commits November 27, 2018 23:24

Fix ES integration test build

4bdb4e5

Version check in dashboards/es_loader_test

6f3cc18

Fix load integration test (improve support for different ES version)

a06b5fa

Fix version check < 7

2e74927

Fix example in metricbeat

222ffaf

Use _doc in metricbeat export template test

847c1c6

urso force-pushed the doc-type-for-7 branch from 0fc79ee to 847c1c6 Compare November 27, 2018 22:24

urso added 2 commits November 28, 2018 15:02

Fix rebase

1534716

ups

2651c5a

ruflin approved these changes Nov 29, 2018

View reviewed changes

Fixup another rebase error

3899a3e

urso merged commit 41f87a4 into elastic:master Nov 29, 2018

This was referenced Nov 29, 2018

Use type _doc instead of doc for elasticsearch >= 7.0 #7612

Closed

Update beats framework to a4b06ae elastic/apm-server#1615

Merged

urso mentioned this pull request Dec 5, 2018

Cherry-pick #9056 to 6.x: Use _doc if ES major version is 7 #9390

Closed

urso added the v6.6.0 label Dec 5, 2018

graphaelli added a commit to graphaelli/apm-server that referenced this pull request Dec 14, 2018

backport/sync elastic/beats#9056

0a8a1cd

ycombinator mentioned this pull request Dec 27, 2018

[6.x] Changing version fields/vars datatypes from string to common.Version #9813

Merged

ycombinator mentioned this pull request Dec 28, 2018

Cherry-pick #9813 to 6.6: [6.x] Changing version fields/vars datatypes from string to common.Version #9815

Merged

henrikno mentioned this pull request Jan 11, 2019

Beats can't to write to older versions of ES than 6.2 due to _doc #10012

Closed

urso deleted the doc-type-for-7 branch February 19, 2019 18:56

Conversation

urso commented Nov 13, 2018

Uh oh!

Uh oh!

ruflin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ruflin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jsoriano Nov 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

urso commented Nov 20, 2018

Uh oh!

ruflin commented Nov 23, 2018

Uh oh!

urso commented Nov 23, 2018

Uh oh!

ruflin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jsoriano Nov 19, 2018 •

edited

Loading