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 monitoring_type here instead of _type #32506

Conversation

chrisronline
Copy link
Contributor

@chrisronline chrisronline commented Mar 5, 2019

Resolves #32503

BLOCKED BY #32515

From the ticket linked in the issue:

@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)

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

See issue for more details.

Until the ES changes are in, this is an expected warning in the kibana server log:

[warning][kibana-monitoring][monitoring] [illegal_argument_exception] Action/metadata line [1] contains an unknown parameter [monitoring_type]

@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jakelandis
Copy link
Contributor

@chrisronline
The recommendation was to remove the type from the JSON bulk payload in favor of an HTTP parameter. See: #32458 (comment)

However, I am only just now realizing that with that change as I recommended you can no longer send multiple types within the same bulk request. i.e. each bulk request would need to have the same type, which (likely) changes things much more then intended. Can you confirm/deny that multiple types may be sent in the same bulk request ?

@chrisronline
Copy link
Contributor Author

Nice catch @jakelandis!

Can you confirm/deny that multiple types may be sent in the same bulk request ?

Yes, I can confirm multiple types may be sent.

@jakelandis
Copy link
Contributor

@chrisronline - in that case, the only way I can see to resolve this is for Kibana (and LS/Beats) to send the type as part of the JSON payload of document instead of the JSON bulk instruction.

Specifically adding the type here: https://github.com/elastic/elasticsearch/blob/master/x-pack/plugin/core/src/main/resources/monitoring-kibana.json#L25

ES will need to be updated to ignore the _type if the type is already present in the payload.

thoughts ?

@chrisronline
Copy link
Contributor Author

@jakelandis Yes that makes sense.

For compatibility, we'd still want ES to read this the _type parameter (if present) so we can ensure 6.7 and below will work for 7.0+ ES.

Does that introduce too much headache on your side to do that?

@chrisronline chrisronline reopened this Mar 5, 2019
@chrisronline
Copy link
Contributor Author

Update here.

We will provide the type in the document payload, but we are blocked by a few things listed in #32515

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@chrisronline
Copy link
Contributor Author

We won't be doing this work right now in Kibana.

@chrisronline chrisronline deleted the monitoring/monitoring_type_bulk_uploader branch March 6, 2019 17:36
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.

3 participants