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

[Deprecation Warnings] x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js #32458

Closed
alexwizp opened this issue Mar 5, 2019 · 8 comments
Assignees
Labels
deprecation_warnings release_note:skip Skip the PR/issue when compiling release notes Team:Monitoring Stack Monitoring team v7.0.0 v7.2.0 v8.0.0

Comments

@alexwizp
Copy link
Contributor

alexwizp commented Mar 5, 2019

Fix deprecation warning message for the following code:

image

This will actually break internal collection. It's a bit confusing because the endpoint we use to index monitoring documents is a different endpoint than the typical bulk endpoint. This _type is actually rewritten in this custom ES endpoint to not be the type of the document, but rather a type field nested in the source of the indexed document. We need to remove this change.

Originally posted by @chrisronline in #32315

@alexwizp alexwizp changed the title [Deprecation Warnings] This will actually break internal collection. It's a bit confusing because the endpoint we use to index monitoring documents is a different endpoint than the typical bulk endpoint. This _type is actually rewritten in this custom ES endpoint to not be the type of the document, but rather a type field nested in the source of the indexed document. We need to remove this change. [Deprecation Warnings] x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js Outdated Mar 5, 2019
@alexwizp alexwizp changed the title [Deprecation Warnings] x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js Outdated [Deprecation Warnings] x-pack/plugins/monitoring/server/kibana_monitoring/bulk_uploader.js Mar 5, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring

@alexwizp
Copy link
Contributor Author

alexwizp commented Mar 5, 2019

@chrisronline @tsullivan could you please help us with this issue or give us more details? We couldn't find broken cases if we remove '_type' from the 'bulk' request.

Below you can find more information about the request which we send to the ES if you need

image

@chrisronline
Copy link
Contributor

I think this needs to be fixed in ES.

This is the deprecation log which is connected to the monitoring bulk endpoint through RestMonitoringBulkAction -> XPackRestHandler. We might need to add some logic to avoid the deprecation warning for the monitoring rest action.

Pinging @jakelandis about this. I'm not sure if you are the best contact, but maybe you know who might be. Thanks!

@jakelandis
Copy link
Contributor

@alexwizp and @chrisronline are correct that this will break monitoring since ES converts that _type to the type inside the document. Any means to avoid that deprecation warning is hacky at best and won't work 8.0.

Could we instead of sending a _type send an additional parameter monitoring_type , and I will add support for that parameter in v7.0.0+. (note - we can not use type as the parameter name since that is an existing parameter (and unused) to confusingly enough actually change the _type)

The proposed change here would result in request like this:

/_monitoring/bulk?system=kibana&system_api_version=6&interval=1000ms&monitoring_type=kibana_stats
{"index": {}{
{"kibana": { ....

This change will only work once a change is made to ES, which I will do ASAP.

Note - We will want to update Logstash and Beats to this as well since they have the same issue.

@chrisronline
Copy link
Contributor

Thanks for chiming in @jakelandis.

If we get this change in for 7.0 for Kibana, Logstash, and Beats, are we okay with the experience for users on <= 6.7, assuming they are on ES 7.0? They'll basically continue to see the deprecation logs (which aren't actionable for them directly) until they upgrade those products to 7.0

pinging @elastic/stack-monitoring team here too

@jakelandis
Copy link
Contributor

are we okay with the experience for users on <= 6.7, assuming they are on ES 7.0

Yes. This isn't the only case of a 6.7 cluster -> 7.0 triggering deprecation warnings. Fortunately none of them are of the spamming variety and will only show up once per start of ES.

@chrisronline
Copy link
Contributor

@jakelandis Thanks. Opened #32503

@chrisronline
Copy link
Contributor

From this comment, the ES team will:

We will silence the deprecation warning in ES code for the _type on monitoring/_bulk endpoint.

I'm going to close this ticket and link the ES ticket (once opened by @jakelandis) here

@rayafratkina rayafratkina added the release_note:skip Skip the PR/issue when compiling release notes label Mar 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation_warnings release_note:skip Skip the PR/issue when compiling release notes Team:Monitoring Stack Monitoring team v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Development

No branches or pull requests

6 participants