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

Remove types from Monitoring plugin "backend" code #37745

Merged
merged 16 commits into from
Feb 4, 2019
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,6 @@ private byte[] toBulkBytes(final MonitoringDoc doc) throws IOException {
builder.startObject("index");
{
builder.field("_index", index);
builder.field("_type", "doc");
if (id != null) {
builder.field("_id", id);
}
Expand All @@ -162,7 +161,10 @@ private byte[] toBulkBytes(final MonitoringDoc doc) throws IOException {
// Adds final bulk separator
out.write(xContent.streamSeparator());

logger.trace("added index request [index={}, type={}, id={}]", index, doc.getType(), id);
logger.trace(
"http exporter [{}] - added index request [index={}, id={}, monitoring data type={}]",
name, index, id, doc.getType()
);

return BytesReference.toBytes(out.bytes());
} catch (Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public void doAdd(Collection<MonitoringDoc> docs) throws ExportException {
try {
final String index = MonitoringTemplateUtils.indexName(formatter, doc.getSystem(), doc.getTimestamp());

final IndexRequest request = new IndexRequest(index, "doc");
final IndexRequest request = new IndexRequest(index);
if (Strings.hasText(doc.getId())) {
request.id(doc.getId());
}
Expand All @@ -82,8 +82,8 @@ public void doAdd(Collection<MonitoringDoc> docs) throws ExportException {
requestBuilder.add(request);

if (logger.isTraceEnabled()) {
logger.trace("local exporter [{}] - added index request [index={}, type={}, id={}, pipeline={}]",
name, request.index(), request.type(), request.id(), request.getPipeline());
logger.trace("local exporter [{}] - added index request [index={}, id={}, pipeline={}, monitoring data type={}]",
name, request.index(), request.id(), request.getPipeline(), doc.getType());
}
} catch (Exception e) {
if (exception == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,6 @@
"add_to_alerts_index": {
"index": {
"index": ".monitoring-alerts-6",
"doc_type": "doc",
ycombinator marked this conversation as resolved.
Show resolved Hide resolved
"doc_id": "${monitoring.watch.unique_id}"
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,7 @@
"actions": {
"add_to_alerts_index": {
"index": {
"index": ".monitoring-alerts-6",
"doc_type": "doc"
"index": ".monitoring-alerts-6"
}
},
"send_email_to_admin": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@
"add_to_alerts_index": {
"index": {
"index": ".monitoring-alerts-6",
"doc_type": "doc",
"doc_id": "${monitoring.watch.unique_id}"
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@
"add_to_alerts_index": {
"index": {
"index": ".monitoring-alerts-6",
"doc_type": "doc",
"doc_id": "${monitoring.watch.unique_id}"
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@
"add_to_alerts_index": {
"index": {
"index": ".monitoring-alerts-6",
"doc_type": "doc",
"doc_id": "${monitoring.watch.unique_id}"
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@
"add_to_alerts_index": {
"index": {
"index": ".monitoring-alerts-6",
"doc_type": "doc",
"doc_id": "${monitoring.watch.unique_id}"
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ public void testAddRequestContentWithUnrecognizedIndexName() throws IOException
assertThat(e.getMessage(), containsString("unrecognized index name [" + indexName + "]"));
//This test's JSON contains outdated references to types
assertWarnings(RestBulkAction.TYPES_DEPRECATION_MESSAGE);

}

public void testSerialization() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,6 @@ private void checkMonitoringDocs() {
assertTrue("document is missing cluster_uuid field", Strings.hasText((String) source.get("cluster_uuid")));
assertTrue("document is missing timestamp field", Strings.hasText(timestamp));
assertTrue("document is missing type field", Strings.hasText(type));
assertEquals("document _type is 'doc'", "doc", hit.getType());

@SuppressWarnings("unchecked")
Map<String, Object> docSource = (Map<String, Object>) source.get("doc");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,14 @@ private static BytesReference generateTemplateSource(final String name, final In
.field("index.number_of_replicas", 0)
.endObject()
.startObject("mappings")
// Still need use type, RestPutIndexTemplateAction#prepareRequestSource has logic that adds type if missing
.startObject("doc")
.startObject("_source")
.field("enabled", false)
.endObject()
.startObject("_meta")
.field("test", true)
.endObject()
.field("enabled", false)
.endObject()
.endObject();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.license.License;
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.rest.action.document.RestBulkAction;
import org.elasticsearch.search.SearchHit;
import org.elasticsearch.search.SearchHits;
import org.elasticsearch.search.collapse.CollapseBuilder;
Expand Down Expand Up @@ -104,11 +103,11 @@ protected Collection<Class<? extends Plugin>> getPlugins() {
}

private String createBulkEntity() {
return "{\"index\":{\"_type\":\"test\"}}\n" +
return "{\"index\":{}}\n" +
"{\"foo\":{\"bar\":0}}\n" +
"{\"index\":{\"_type\":\"test\"}}\n" +
"{\"index\":{}}\n" +
"{\"foo\":{\"bar\":1}}\n" +
"{\"index\":{\"_type\":\"test\"}}\n" +
"{\"index\":{}}\n" +
"{\"foo\":{\"bar\":2}}\n" +
"\n";
}
Expand All @@ -127,7 +126,7 @@ public void testMonitoringBulk() throws Exception {

final MonitoringBulkResponse bulkResponse =
new MonitoringBulkRequestBuilder(client())
.add(system, null, new BytesArray(createBulkEntity().getBytes("UTF-8")), XContentType.JSON,
.add(system, "monitoring_data_type", new BytesArray(createBulkEntity().getBytes("UTF-8")), XContentType.JSON,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add in a default type here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pickypg can correct me if I'm wrong but I believe this argument is the monitoring data type, e.g. beats_stats or logstash_state, etc. that are sent in to the POST _xpack/monitoring/_bulk API. So I wanted to be more explicit about what the type is, so we don't confuse it in the future with _type.

Copy link
Member

Choose a reason for hiding this comment

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

They are. We use that as a mechanism for routing the document to the right index.

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, i see now.

System.currentTimeMillis(), interval.millis())
.get();

Expand Down Expand Up @@ -178,10 +177,9 @@ public void testMonitoringBulk() throws Exception {
equalTo(1L));

for (final SearchHit hit : hits.getHits()) {
assertMonitoringDoc(toMap(hit), system, "test", interval);
assertMonitoringDoc(toMap(hit), system, "monitoring_data_type", interval);
}
});
assertWarnings(RestBulkAction.TYPES_DEPRECATION_MESSAGE);
}

/**
Expand Down Expand Up @@ -264,7 +262,6 @@ private void assertMonitoringDoc(final Map<String, Object> document,

final String index = (String) document.get("_index");
assertThat(index, containsString(".monitoring-" + expectedSystem.getSystem() + "-" + TEMPLATE_VERSION + "-"));
assertThat(document.get("_type"), equalTo("doc"));
assertThat((String) document.get("_id"), not(isEmptyOrNullString()));

final Map<String, Object> source = (Map<String, Object>) document.get("_source");
Expand Down