honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Watcher: Improve http attachment history information (elastic/elasticsearch#3436) 

When the HTTP attachment was not able to successfully retrieve the
data from and endpoint, there was no indication in the watch history
of what went wrong. Instead a logger was used, which is not useful
for the person running the watches.

This commit removes the logger statement and throws an exception,
so that the exception message can be stored in the watch history.

Source of this issue was a forum post:
https://discuss.elastic.co/t/sending-e-mail-with-generated-report-fails/60263/6

Original commit: elastic/x-pack-elasticsearch@acdaf7abef
This commit is contained in:
Alexander Reelsen 2016-09-12 17:04:22 +02:00 committed by GitHub
parent 5c8ece8583
commit 2a6a9a10f7
3 changed files with 87 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
elasticsearch/x-pack/src/main/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParser.java
View File

@ -6,8 +6,6 @@
package org.elasticsearch.xpack.notification.email.attachment;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.ParseField;
@ -102,25 +100,22 @@ public class HttpEmailAttachementParser implements EmailAttachmentParser<HttpReq
return new Attachment.Bytes(attachment.id(), BytesReference.toBytes(response.body()), attachmentContentType,
attachment.inline());
} else {
logger.error("Empty response body: [host[{}], port[{}], method[{}], path[{}]: response status [{}]", httpRequest.host(),
httpRequest.port(), httpRequest.method(), httpRequest.path(), response.status());
throw new ElasticsearchException("Watch[{}] attachment[{}] HTTP empty response body host[{}], port[{}], " +
"method[{}], path[{}], status[{}]",
context.watch().id(), attachment.id(), httpRequest.host(), httpRequest.port(), httpRequest.method(),
httpRequest.path(), response.status());
}
} else {
logger.error("Error getting http response: [host[{}], port[{}], method[{}], path[{}]: response status [{}]",
httpRequest.host(), httpRequest.port(), httpRequest.method(), httpRequest.path(), response.status());
throw new ElasticsearchException("Watch[{}] attachment[{}] HTTP error status host[{}], port[{}], " +
"method[{}], path[{}], status[{}]",
context.watch().id(), attachment.id(), httpRequest.host(), httpRequest.port(), httpRequest.method(),
httpRequest.path(), response.status());
}
} catch (IOException e) {
logger.error(
(Supplier<?>) () -> new ParameterizedMessage(
"Error executing HTTP request: [host[{}], port[{}], method[{}], path[{}]",
httpRequest.host(),
httpRequest.port(),
httpRequest.method(),
httpRequest.path()),
e);
throw new ElasticsearchException("Watch[{}] attachment[{}] Error executing HTTP request host[{}], port[{}], " +
"method[{}], path[{}], exception[{}]",
context.watch().id(), attachment.id(), httpRequest.host(), httpRequest.port(), httpRequest.method(),
httpRequest.path(), e.getMessage());
}
throw new ElasticsearchException("Unable to get attachment of type [{}] with id [{}] in watch [{}] aborting watch execution",
type(), attachment.id(), context.watch().id());
}
}

78
elasticsearch/x-pack/src/test/java/org/elasticsearch/xpack/notification/email/attachment/HttpEmailAttachementParserTests.java
View File

@ -5,6 +5,8 @@
*/
package org.elasticsearch.xpack.notification.email.attachment;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.collect.MapBuilder;
import org.elasticsearch.common.xcontent.ToXContent;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
@ -17,9 +19,15 @@ import org.elasticsearch.xpack.common.http.HttpResponse;
import org.elasticsearch.xpack.common.http.auth.HttpAuthRegistry;
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuth;
import org.elasticsearch.xpack.common.http.auth.basic.BasicAuthFactory;
import org.elasticsearch.xpack.watcher.execution.WatchExecutionContext;
import org.elasticsearch.xpack.watcher.execution.Wid;
import org.elasticsearch.xpack.watcher.test.MockTextTemplateEngine;
import org.elasticsearch.xpack.watcher.watch.Payload;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.junit.Before;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
@ -28,6 +36,7 @@ import java.util.Map;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Collections.singletonMap;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.xpack.watcher.test.WatcherTestUtils.mockExecutionContextBuilder;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.core.Is.is;
import static org.mockito.Matchers.any;
@ -38,6 +47,8 @@ public class HttpEmailAttachementParserTests extends ESTestCase {
private HttpRequestTemplate.Parser httpRequestTemplateParser;
private HttpClient httpClient;
private EmailAttachmentsParser emailAttachmentsParser;
private Map<String, EmailAttachmentParser> attachmentParsers;
@Before
public void init() throws Exception {
@ -45,16 +56,16 @@ public class HttpEmailAttachementParserTests extends ESTestCase {
httpRequestTemplateParser = new HttpRequestTemplate.Parser(authRegistry);
httpClient = mock(HttpClient.class);
HttpResponse response = new HttpResponse(200, "This is my response".getBytes(UTF_8));
when(httpClient.execute(any(HttpRequest.class))).thenReturn(response);
attachmentParsers = new HashMap<>();
attachmentParsers.put(HttpEmailAttachementParser.TYPE,
new HttpEmailAttachementParser(httpClient, httpRequestTemplateParser, new MockTextTemplateEngine()));
emailAttachmentsParser = new EmailAttachmentsParser(attachmentParsers);
}
public void testSerializationWorks() throws Exception {
Map<String, EmailAttachmentParser> attachmentParsers = new HashMap<>();
attachmentParsers.put(HttpEmailAttachementParser.TYPE,
new HttpEmailAttachementParser(httpClient, httpRequestTemplateParser, new MockTextTemplateEngine()));
EmailAttachmentsParser emailAttachmentsParser = new EmailAttachmentsParser(attachmentParsers);
HttpResponse response = new HttpResponse(200, "This is my response".getBytes(UTF_8));
when(httpClient.execute(any(HttpRequest.class))).thenReturn(response);
String id = "some-id";
XContentBuilder builder = jsonBuilder().startObject().startObject(id)
@ -91,4 +102,59 @@ public class HttpEmailAttachementParserTests extends ESTestCase {
assertThat(attachments.get(0).inline(), is(isInline));
}
public void testNonOkHttpCodeThrowsException() throws Exception {
HttpResponse response = new HttpResponse(403, "This is my response".getBytes(UTF_8));
when(httpClient.execute(any(HttpRequest.class))).thenReturn(response);
HttpRequestTemplate requestTemplate = HttpRequestTemplate.builder("localhost", 80).path("foo").build();
HttpRequestAttachment attachment = new HttpRequestAttachment("someid", requestTemplate, false, null);
WatchExecutionContext ctx = createWatchExecutionContext();
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> attachmentParsers.get(HttpEmailAttachementParser.TYPE).toAttachment(ctx, new Payload.Simple(), attachment));
assertThat(exception.getMessage(), is("Watch[watch1] attachment[someid] HTTP error status host[localhost], port[80], " +
"method[GET], path[foo], status[403]"));
}
public void testEmptyResponseThrowsException() throws Exception {
HttpResponse response = new HttpResponse(200);
when(httpClient.execute(any(HttpRequest.class))).thenReturn(response);
HttpRequestTemplate requestTemplate = HttpRequestTemplate.builder("localhost", 80).path("foo").build();
HttpRequestAttachment attachment = new HttpRequestAttachment("someid", requestTemplate, false, null);
WatchExecutionContext ctx = createWatchExecutionContext();
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> attachmentParsers.get(HttpEmailAttachementParser.TYPE).toAttachment(ctx, new Payload.Simple(), attachment));
assertThat(exception.getMessage(), is("Watch[watch1] attachment[someid] HTTP empty response body host[localhost], port[80], " +
"method[GET], path[foo], status[200]"));
}
public void testHttpClientThrowsException() throws Exception {
when(httpClient.execute(any(HttpRequest.class))).thenThrow(new IOException("whatever"));
HttpRequestTemplate requestTemplate = HttpRequestTemplate.builder("localhost", 80).path("foo").build();
HttpRequestAttachment attachment = new HttpRequestAttachment("someid", requestTemplate, false, null);
WatchExecutionContext ctx = createWatchExecutionContext();
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> attachmentParsers.get(HttpEmailAttachementParser.TYPE).toAttachment(ctx, new Payload.Simple(), attachment));
assertThat(exception.getMessage(), is("Watch[watch1] attachment[someid] Error executing HTTP request host[localhost], port[80], " +
"method[GET], path[foo], exception[whatever]"));
}
private WatchExecutionContext createWatchExecutionContext() {
DateTime now = DateTime.now(DateTimeZone.UTC);
Wid wid = new Wid(randomAsciiOfLength(5), randomLong(), now);
Map<String, Object> metadata = MapBuilder.<String, Object>newMapBuilder().put("_key", "_val").map();
return mockExecutionContextBuilder("watch1")
.wid(wid)
.payload(new Payload.Simple())
.time("watch1", now)
.metadata(metadata)
.buildMock();
}
}

6
elasticsearch/x-pack/watcher/src/test/java/org/elasticsearch/xpack/watcher/actions/email/EmailActionTests.java
View File

@ -510,7 +510,7 @@ public class EmailActionTests extends ESTestCase {
public void testThatOneFailedEmailAttachmentResultsInActionFailure() throws Exception {
EmailService emailService = new AbstractWatcherIntegrationTestCase.NoopEmailService();
TextTemplateEngine engine = mock(TextTemplateEngine.class);
TextTemplateEngine engine = new MockTextTemplateEngine();
HttpClient httpClient = mock(HttpClient.class);
// setup mock response, second one is an error
@ -542,7 +542,6 @@ public class EmailActionTests extends ESTestCase {
.endObject()
.endObject();
XContentParser parser = JsonXContent.jsonXContent.createParser(builder.bytes());
logger.info("JSON: {}", builder.string());
parser.nextToken();
@ -563,7 +562,8 @@ public class EmailActionTests extends ESTestCase {
assertThat(result, instanceOf(EmailAction.Result.Failure.class));
EmailAction.Result.Failure failure = (EmailAction.Result.Failure) result;
assertThat(failure.reason(),
is("Unable to get attachment of type [http] with id [second] in watch [watch1] aborting watch execution"));
is("Watch[watch1] attachment[second] HTTP error status host[localhost], port[80], method[GET], path[/second], " +
"status[403]"));
}
private EmailActionFactory createEmailActionFactory() {