Remove types from internal monitoring templates and bump to api 7 #39888

jakelandis · 2019-03-10T18:46:07Z

This commit removes the "doc" type from monitoring internal indexes.
The template still carries the "_doc" type since that is needed for
the internal representation.

This change impacts the following templates:
monitoring-alerts.json
monitoring-beats.json
monitoring-es.json
monitoring-kibana.json
monitoring-logstash.json

As part of the required changes, the system_api_version has been
bumped from "6" to "7" and support for version "2" has been dropped.

A new empty pipeline is now introduced for the version "7", and
the formerly empty "6" pipeline will now remove the type and re-direct
the request to the "7" index.

Additionally, to due to a difference in the internal representation
(which requires the inclusion of "_doc" type) and external representation
(which requires the exclusion of any type) a helper method is introduced
to help convert internal to external representation, and used by the
monitoring HTTP template exporter.

Relates #38637

This replaces #39336 , the "primary change" is still relevant for this PR, but the additional changes on that PR are not relevant to this PR since we decided to go a different way that is more inline with prior upgrades.

The template still carries the "_doc" type since that is needed for the internal representation. This change impacts the following templates: monitoring-alerts.json monitoring-beats.json monitoring-es.json monitoring-kibana.json monitoring-logstash.json As part of the required changes, the system_api_version has been bumped from "6" to "7" and support for version "2" has been dropped. A new empty pipeline is now introduced for the version "7", and the formerly empty "6" pipeline will now remove the type and re-direct the request to the "7" index. Additionally, to due to a difference in the internal representation (which requires the inclusion of "_doc" type) and external representation (which requires the exclusion of any type) a helper method is introduced to help convert internal to external representation, and used by the monitoring HTTP template exporter. Relates elastic#38637

elasticmachine · 2019-03-10T18:48:35Z

Pinging @elastic/es-core-features

jakelandis · 2019-03-10T22:57:10Z

@elasticmachine run elasticsearch-ci/bwc

jakelandis · 2019-03-11T02:22:19Z

@elasticmachine run elasticsearch-ci/bwc

jpountz · 2019-03-11T09:20:02Z

...ing/src/main/java/org/elasticsearch/xpack/monitoring/exporter/http/TemplateHttpResource.java

@@ -98,8 +104,18 @@ protected void doPublish(final RestClient client, final ActionListener<Boolean>
     *
     * @return Never {@code null}.
     */
+    @SuppressWarnings("unchecked")


what is the call that requires suppressing this warning?

Good catch, that was from an earlier attempt and is no longer needed. It is now removed.

jpountz · 2019-03-11T09:22:39Z

...g/src/main/java/org/elasticsearch/xpack/monitoring/rest/action/RestMonitoringBulkAction.java

@@ -75,7 +76,9 @@ public String getName() {

    @Override
    public RestChannelConsumer doPrepareRequest(RestRequest request, XPackClient client) throws IOException {
-        final String defaultType = request.param("type");
+        if (Strings.isEmpty(request.param("type")) == false) {
+            logger.error("Custom types for monitoring is not supported");


Why are we logging instead of failing with an exception? Are we sure that 6.7 never uses the bulk monitoring API with a type in the path? If not I think we should keep support for the defaultType for now and remove it in 8.0. If we are sure that the type is never set via the path, then maybe we should remove the path from the REST handler entirely?

I am like 99% sure that Kibana/Beats/LS don't send this parameter. I went ahead and moved to an throwing an exception if it is sent.

Should we instead remove the paths that accept a type in the constructor of RestMonitoringBulkAction?

Ahh, yes (sorry a bit slow here). I removed this on 1573aae.

jpountz

LGTM

jakelandis · 2019-03-11T16:05:30Z

@jpountz thanks !

@pickypg @ycombinator - I am going to merge this based on Adrien's review, apologies for not providing adequate time for review (trying to meet specific timelines) If you do you review and find issue(s), please ping me directly.

…astic#39888) This commit removes the "doc" type from monitoring internal indexes. The template still carries the "_doc" type since that is needed for the internal representation. This change impacts the following templates: monitoring-alerts.json monitoring-beats.json monitoring-es.json monitoring-kibana.json monitoring-logstash.json As part of the required changes, the system_api_version has been bumped from "6" to "7" and support for version "2" has been dropped. A new empty pipeline is now introduced for the version "7", and the formerly empty "6" pipeline will now remove the type and re-direct the request to the "7" index. Additionally, to due to a difference in the internal representation (which requires the inclusion of "_doc" type) and external representation (which requires the exclusion of any type) a helper method is introduced to help convert internal to external representation, and used by the monitoring HTTP template exporter. Relates elastic#38637

…9888) (#39927) This commit removes the "doc" type from monitoring internal indexes. The template still carries the "_doc" type since that is needed for the internal representation. This change impacts the following templates: monitoring-alerts.json monitoring-beats.json monitoring-es.json monitoring-kibana.json monitoring-logstash.json As part of the required changes, the system_api_version has been bumped from "6" to "7" and support for version "2" has been dropped. A new empty pipeline is now introduced for the version "7", and the formerly empty "6" pipeline will now remove the type and re-direct the request to the "7" index. Additionally, to due to a difference in the internal representation (which requires the inclusion of "_doc" type) and external representation (which requires the exclusion of any type) a helper method is introduced to help convert internal to external representation, and used by the monitoring HTTP template exporter. Relates #38637

elastic/elasticsearch#39888 removed the "doc" type from internal indices, causing breakage in the x-pack integration tests. This commit changes the type to "_doc" for consistency.

…9888) (#39926) This commit removes the "doc" type from monitoring internal indexes. The template still carries the "_doc" type since that is needed for the internal representation. This change impacts the following templates: monitoring-alerts.json monitoring-beats.json monitoring-es.json monitoring-kibana.json monitoring-logstash.json As part of the required changes, the system_api_version has been bumped from "6" to "7" and support for version "2" has been dropped. A new empty pipeline is now introduced for the version "7", and the formerly empty "6" pipeline will now remove the type and re-direct the request to the "7" index. Additionally, to due to a difference in the internal representation (which requires the inclusion of "_doc" type) and external representation (which requires the exclusion of any type) a helper method is introduced to help convert internal to external representation, and used by the monitoring HTTP template exporter. Relates #38637

This commit fixes x-pack integration tests that were broken by elastic/elasticsearch#39888 removing the "doc" type, and using `_doc` in templates.

This commit fixes x-pack integration tests that were broken by elastic/elasticsearch#39888 removing the "doc" type, and using `_doc` in templates. Fixes #10533

org.elasticsearch.xpack.monitoring.action.MonitoringBulkRequestTests#testAddRequestContent can still randomly use a defaultType for monitoring. The defaultType support has been removed as of PR elastic#39888. Prior to its's removal it would default the type if one is not specified. The _type on the monitoring bulk end point is currently required, though it is not used as the final index type (which defaultType would have). Closes elastic#39980

org.elasticsearch.xpack.monitoring.action.MonitoringBulkRequestTests#testAddRequestContent can still randomly use a defaultType for monitoring. The defaultType support has been removed as of PR #39888. Prior to its's removal it would default the type if one is not specified. The _type on the monitoring bulk end point is currently required, though it is not used as the final index type (which defaultType would have). Closes #39980

jakelandis changed the title ~~This commit removes the "doc" type from monitoring internal indexes.~~ Remove types from internal monitoring templates and bump to api 7 Mar 10, 2019

jakelandis mentioned this pull request Mar 10, 2019

wip: monitoring template type removal #39336

Closed

jakelandis added v7.0.0 :Data Management/Monitoring v8.0.0 v7.2.0 labels Mar 10, 2019

jakelandis requested review from ycombinator, pickypg and jpountz March 10, 2019 18:48

jakelandis mentioned this pull request Mar 10, 2019

Remove types from index templates for x-pack features #38637

Closed

4 tasks

jakelandis added 2 commits March 10, 2019 15:04

fix mapping paths for tests

9fbe98b

fix line length of 141

28d19c8

jpountz reviewed Mar 11, 2019

View reviewed changes

jakelandis added 4 commits March 11, 2019 07:34

review comments

5468b90

Merge branch 'master' into type_removal_monitoring

683e3c3

minor correction

f2a623d

remove type from deprecrated path

1573aae

jpountz approved these changes Mar 11, 2019

View reviewed changes

jakelandis merged commit b5cb845 into elastic:master Mar 11, 2019

jakelandis added the backport pending label Mar 11, 2019

jakelandis removed the backport pending label Mar 11, 2019

robbavey mentioned this pull request Mar 11, 2019

Change internal document type to push "_doc" instead of "doc" elastic/logstash#10533

Closed

andyb-elastic mentioned this pull request Mar 12, 2019

[CI] MonitoringBulkRequestTests.testAddRequestContent #39980

Closed

jakelandis mentioned this pull request Mar 13, 2019

Fix test which still uses default type #39997

Merged

chrisronline mentioned this pull request Apr 2, 2019

[Monitoring] 6.7 Kibana/ES does not work with ES 7.0 elastic/kibana#34357

Closed

jakelandis added v7.0.0-rc2 and removed v7.0.0 labels Apr 3, 2019

colings86 added the >non-issue label Jun 18, 2019

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove types from internal monitoring templates and bump to api 7 #39888

Remove types from internal monitoring templates and bump to api 7 #39888

jakelandis commented Mar 10, 2019 •

edited

Loading

elasticmachine commented Mar 10, 2019

jakelandis commented Mar 10, 2019

jakelandis commented Mar 11, 2019

jpountz Mar 11, 2019

jakelandis Mar 11, 2019

jpountz Mar 11, 2019

jakelandis Mar 11, 2019

jpountz Mar 11, 2019

jakelandis Mar 11, 2019

jpountz left a comment

jakelandis commented Mar 11, 2019

Remove types from internal monitoring templates and bump to api 7 #39888

Remove types from internal monitoring templates and bump to api 7 #39888

Conversation

jakelandis commented Mar 10, 2019 • edited Loading

elasticmachine commented Mar 10, 2019

jakelandis commented Mar 10, 2019

jakelandis commented Mar 11, 2019

jpountz Mar 11, 2019

Choose a reason for hiding this comment

jakelandis Mar 11, 2019

Choose a reason for hiding this comment

jpountz Mar 11, 2019

Choose a reason for hiding this comment

jakelandis Mar 11, 2019

Choose a reason for hiding this comment

jpountz Mar 11, 2019

Choose a reason for hiding this comment

jakelandis Mar 11, 2019

Choose a reason for hiding this comment

jpountz left a comment

Choose a reason for hiding this comment

jakelandis commented Mar 11, 2019

jakelandis commented Mar 10, 2019 •

edited

Loading