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

[Monitoring] Use op_type = create with ES version >= 7.5.0 #14313

Merged
merged 4 commits into from
Nov 1, 2019
Merged

[Monitoring] Use op_type = create with ES version >= 7.5.0 #14313

merged 4 commits into from
Nov 1, 2019

Conversation

ycombinator
Copy link
Contributor

@ycombinator ycombinator commented Oct 29, 2019

Follow up to #13936.

In #13936, we taught the Elasticsearch output client in libbeat to use the create op_type in bulk requests to Elasticsearch if the Elasticsearch cluster's version was 7.5.0 or newer. This PR similarly teaches the Elasticsearch monitoring client to do the same.

Here's what the bulk request from the monitoring client looks like with Elasticsearch 7.4.0:

7_4_0

And here's what it looks like with Elasticsearch 8.0.0:

8_0_0

@elasticmachine
Copy link
Collaborator

Pinging @elastic/stack-monitoring (Stack monitoring)

@ycombinator ycombinator requested review from urso and ph October 29, 2019 19:22
@ycombinator ycombinator marked this pull request as ready for review October 29, 2019 19:25
@ycombinator ycombinator changed the title Use op_type = create with ES version >= 7.5.0 [Monitoring] Use op_type = create with ES version >= 7.5.0 Oct 29, 2019
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM

"index": meta,
var action common.MapStr
var opType string
if esVersion.LessThan(createDocPrivAvailableESVersion) {
Copy link
Contributor

Choose a reason for hiding this comment

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

didn't know about lessThan make it a lot cleaner.

@ycombinator
Copy link
Contributor Author

CI failures look related to this PR. Will investigate and fix.

@ycombinator ycombinator added in progress Pull request is currently in progress. v7.5.1 and removed review v7.5.1 labels Oct 30, 2019
@ycombinator
Copy link
Contributor Author

I was unable to reproduce the CI failures, specifically the test_monitoring.py system test in libbeat, locally. I compared the SHA256 digests of the elasticsearch:8.0.0-SNAPSHOT image that was downloaded to my local machine with the one downloaded by CI and they are the same: 3be50a3164826679d3a9aa7aab15e45de75d87f6be5d3876982b042eb45a3862. So I'm not sure why this is only failing on CI. 🤔

@ycombinator
Copy link
Contributor Author

jenkins, test this

@ycombinator
Copy link
Contributor Author

Now the error seems to have disappeared from CI too. I want to make sure this is not some kind of flakiness in the test, so I'm going to re-run CI a few times on this PR.

@ycombinator
Copy link
Contributor Author

jenkins, test this

1 similar comment
@ycombinator
Copy link
Contributor Author

jenkins, test this

@ycombinator
Copy link
Contributor Author

Travis CI is green. Jenkins CI is red because of x-pack/agent builds being yellow, which is unrelated to this PR. Merging.

@ycombinator ycombinator merged commit abbe7fb into elastic:master Nov 1, 2019
@ycombinator ycombinator deleted the lb-mon-bulk-optype-create branch November 1, 2019 18:46
ycombinator added a commit that referenced this pull request Nov 7, 2019
…14372)

* Use op_type = create with ES version >= 7.5.0

* Initialize map

* Printing out logs for debugging

* Removing debugging logs
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
…4313) (elastic#14372)

* Use op_type = create with ES version >= 7.5.0

* Initialize map

* Printing out logs for debugging

* Removing debugging logs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants