You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2020/08/06 19:33:23 UTC
[GitHub] [incubator-pinot] vincentchenjl opened a new pull request #5823: [TE] enchance anomaly api to propagate feedback
vincentchenjl opened a new pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823
This PR is to enhance the anomaly feedback api so that it propagates the feedback to the parent of the specified anomaly and its siblings if the `propagate` flag is set.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] cecilynie commented on a change in pull request #5823: [TE] enchance anomaly api to propagate feedback
Posted by GitBox <gi...@apache.org>.
cecilynie commented on a change in pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823#discussion_r467184126
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -156,30 +156,49 @@ public MergedAnomalyResultDTO getMergedAnomalyDetail(
* eg. payload
* <p/>
* { "feedbackType": "NOT_ANOMALY", "comment": "this is not an anomaly" }
+ * @param propagate : a flag whether it should propagate the same feedback value to the parent of this anomaly
+ * and all its siblings if they exist
*/
@POST
@Path(value = "anomaly-merged-result/feedback/{anomaly_merged_result_id}")
@ApiOperation("update anomaly merged result feedback")
- public void updateAnomalyMergedResultFeedback(@PathParam("anomaly_merged_result_id") long anomalyResultId,
+ public void updateAnomalyMergedResultFeedback(
+ @PathParam("anomaly_merged_result_id") long anomalyResultId,
+ @QueryParam("propagate") @DefaultValue("true") boolean propagate,
Review comment:
Hi Vincent, is the same API used for both anomalies page, root cause page, application page and potentially alert page? Let's make sure the consistent behavior, if users update the children anomalies' feedback from any page above, it will always propagate the feedbacks to the parent anomaly?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jihaozh commented on a change in pull request #5823: [TE] enchance anomaly api to propagate feedback
Posted by GitBox <gi...@apache.org>.
jihaozh commented on a change in pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823#discussion_r466705333
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/datalayer/bao/jdbc/MergedAnomalyResultManagerImpl.java
##########
@@ -456,6 +456,24 @@ public void updateAnomalyFeedback(MergedAnomalyResultDTO entity) {
return this.getAnomaliesForMetricBeanAndTimeRange(mbean, start, end);
}
+ @Override
+ public MergedAnomalyResultDTO findParent(MergedAnomalyResultDTO entity) {
+ List<MergedAnomalyResultBean> candidates = genericPojoDao.get(Predicate.AND(
+ Predicate.EQ("detectionConfigId", entity.getDetectionConfigId()),
+ Predicate.LE("startTime", entity.getStartTime()),
Review comment:
we may need to consider the potential merging gap between the parent and children anomaly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jasonyanwenl commented on a change in pull request #5823: [TE] enchance anomaly api to propagate feedback
Posted by GitBox <gi...@apache.org>.
jasonyanwenl commented on a change in pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823#discussion_r467216505
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -156,30 +156,49 @@ public MergedAnomalyResultDTO getMergedAnomalyDetail(
* eg. payload
* <p/>
* { "feedbackType": "NOT_ANOMALY", "comment": "this is not an anomaly" }
+ * @param propagate : a flag whether it should propagate the same feedback value to the parent of this anomaly
+ * and all its siblings if they exist
*/
@POST
@Path(value = "anomaly-merged-result/feedback/{anomaly_merged_result_id}")
@ApiOperation("update anomaly merged result feedback")
- public void updateAnomalyMergedResultFeedback(@PathParam("anomaly_merged_result_id") long anomalyResultId,
+ public void updateAnomalyMergedResultFeedback(
+ @PathParam("anomaly_merged_result_id") long anomalyResultId,
+ @QueryParam("propagate") @DefaultValue("true") boolean propagate,
String payload) {
try {
MergedAnomalyResultDTO result = anomalyMergedResultDAO.findById(anomalyResultId);
if (result == null) {
throw new IllegalArgumentException("AnomalyResult not found with id " + anomalyResultId);
}
AnomalyFeedbackDTO feedbackRequest = OBJECT_MAPPER.readValue(payload, AnomalyFeedbackDTO.class);
- AnomalyFeedback feedback = result.getFeedback();
- if (feedback == null) {
- feedback = new AnomalyFeedbackDTO();
- result.setFeedback(feedback);
- }
- feedback.setComment(feedbackRequest.getComment());
- if (feedbackRequest.getFeedbackType() != null){
- feedback.setFeedbackType(feedbackRequest.getFeedbackType());
+ if (propagate && result.isChild()) {
+ // propagate the feedback to its parent and siblings
+ MergedAnomalyResultDTO parent = anomalyMergedResultDAO.findParent(result);
+ if (parent != null) {
+ updateAnomalyFeedback(parent, feedbackRequest);
+ } else {
+ LOG.warn("Cannot find parent for anomaly : {}, thus only updating the feedback of it", result.getId());
+ updateAnomalyFeedback(result, feedbackRequest);
+ }
+ } else {
+ updateAnomalyFeedback(result, feedbackRequest);
}
- anomalyMergedResultDAO.updateAnomalyFeedback(result);
} catch (IOException e) {
throw new IllegalArgumentException("Invalid payload " + payload, e);
}
}
+
+ private void updateAnomalyFeedback(MergedAnomalyResultDTO anomaly, AnomalyFeedbackDTO newFeedback) {
Review comment:
Just a quick question, where is the sibling anomaly updated? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5823: [TE] enchance anomaly api to propagate feedback
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823#discussion_r467220926
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -156,30 +156,49 @@ public MergedAnomalyResultDTO getMergedAnomalyDetail(
* eg. payload
* <p/>
* { "feedbackType": "NOT_ANOMALY", "comment": "this is not an anomaly" }
+ * @param propagate : a flag whether it should propagate the same feedback value to the parent of this anomaly
+ * and all its siblings if they exist
*/
@POST
@Path(value = "anomaly-merged-result/feedback/{anomaly_merged_result_id}")
@ApiOperation("update anomaly merged result feedback")
- public void updateAnomalyMergedResultFeedback(@PathParam("anomaly_merged_result_id") long anomalyResultId,
+ public void updateAnomalyMergedResultFeedback(
+ @PathParam("anomaly_merged_result_id") long anomalyResultId,
+ @QueryParam("propagate") @DefaultValue("true") boolean propagate,
Review comment:
I confirm that updating feedbacks in anomaly pages, root cause pages, application pages and alert pages will trigger this API call. I don't think there is any other page where we can update the feedback.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] vincentchenjl commented on a change in pull request #5823: [TE] enchance anomaly api to propagate feedback
Posted by GitBox <gi...@apache.org>.
vincentchenjl commented on a change in pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823#discussion_r467272726
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -156,30 +156,49 @@ public MergedAnomalyResultDTO getMergedAnomalyDetail(
* eg. payload
* <p/>
* { "feedbackType": "NOT_ANOMALY", "comment": "this is not an anomaly" }
+ * @param propagate : a flag whether it should propagate the same feedback value to the parent of this anomaly
+ * and all its siblings if they exist
*/
@POST
@Path(value = "anomaly-merged-result/feedback/{anomaly_merged_result_id}")
@ApiOperation("update anomaly merged result feedback")
- public void updateAnomalyMergedResultFeedback(@PathParam("anomaly_merged_result_id") long anomalyResultId,
+ public void updateAnomalyMergedResultFeedback(
+ @PathParam("anomaly_merged_result_id") long anomalyResultId,
+ @QueryParam("propagate") @DefaultValue("true") boolean propagate,
String payload) {
try {
MergedAnomalyResultDTO result = anomalyMergedResultDAO.findById(anomalyResultId);
if (result == null) {
throw new IllegalArgumentException("AnomalyResult not found with id " + anomalyResultId);
}
AnomalyFeedbackDTO feedbackRequest = OBJECT_MAPPER.readValue(payload, AnomalyFeedbackDTO.class);
- AnomalyFeedback feedback = result.getFeedback();
- if (feedback == null) {
- feedback = new AnomalyFeedbackDTO();
- result.setFeedback(feedback);
- }
- feedback.setComment(feedbackRequest.getComment());
- if (feedbackRequest.getFeedbackType() != null){
- feedback.setFeedbackType(feedbackRequest.getFeedbackType());
+ if (propagate && result.isChild()) {
+ // propagate the feedback to its parent and siblings
+ MergedAnomalyResultDTO parent = anomalyMergedResultDAO.findParent(result);
+ if (parent != null) {
+ updateAnomalyFeedback(parent, feedbackRequest);
+ } else {
+ LOG.warn("Cannot find parent for anomaly : {}, thus only updating the feedback of it", result.getId());
+ updateAnomalyFeedback(result, feedbackRequest);
+ }
+ } else {
+ updateAnomalyFeedback(result, feedbackRequest);
}
- anomalyMergedResultDAO.updateAnomalyFeedback(result);
} catch (IOException e) {
throw new IllegalArgumentException("Invalid payload " + payload, e);
}
}
+
+ private void updateAnomalyFeedback(MergedAnomalyResultDTO anomaly, AnomalyFeedbackDTO newFeedback) {
Review comment:
` anomalyMergedResultDAO.updateAnomalyFeedback(anomaly)` actually updates the feedback labels of `anomaly` and all its child anomaly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jasonyanwenl commented on a change in pull request #5823: [TE] enchance anomaly api to propagate feedback
Posted by GitBox <gi...@apache.org>.
jasonyanwenl commented on a change in pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823#discussion_r467216505
##########
File path: thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -156,30 +156,49 @@ public MergedAnomalyResultDTO getMergedAnomalyDetail(
* eg. payload
* <p/>
* { "feedbackType": "NOT_ANOMALY", "comment": "this is not an anomaly" }
+ * @param propagate : a flag whether it should propagate the same feedback value to the parent of this anomaly
+ * and all its siblings if they exist
*/
@POST
@Path(value = "anomaly-merged-result/feedback/{anomaly_merged_result_id}")
@ApiOperation("update anomaly merged result feedback")
- public void updateAnomalyMergedResultFeedback(@PathParam("anomaly_merged_result_id") long anomalyResultId,
+ public void updateAnomalyMergedResultFeedback(
+ @PathParam("anomaly_merged_result_id") long anomalyResultId,
+ @QueryParam("propagate") @DefaultValue("true") boolean propagate,
String payload) {
try {
MergedAnomalyResultDTO result = anomalyMergedResultDAO.findById(anomalyResultId);
if (result == null) {
throw new IllegalArgumentException("AnomalyResult not found with id " + anomalyResultId);
}
AnomalyFeedbackDTO feedbackRequest = OBJECT_MAPPER.readValue(payload, AnomalyFeedbackDTO.class);
- AnomalyFeedback feedback = result.getFeedback();
- if (feedback == null) {
- feedback = new AnomalyFeedbackDTO();
- result.setFeedback(feedback);
- }
- feedback.setComment(feedbackRequest.getComment());
- if (feedbackRequest.getFeedbackType() != null){
- feedback.setFeedbackType(feedbackRequest.getFeedbackType());
+ if (propagate && result.isChild()) {
+ // propagate the feedback to its parent and siblings
+ MergedAnomalyResultDTO parent = anomalyMergedResultDAO.findParent(result);
+ if (parent != null) {
+ updateAnomalyFeedback(parent, feedbackRequest);
+ } else {
+ LOG.warn("Cannot find parent for anomaly : {}, thus only updating the feedback of it", result.getId());
+ updateAnomalyFeedback(result, feedbackRequest);
+ }
+ } else {
+ updateAnomalyFeedback(result, feedbackRequest);
}
- anomalyMergedResultDAO.updateAnomalyFeedback(result);
} catch (IOException e) {
throw new IllegalArgumentException("Invalid payload " + payload, e);
}
}
+
+ private void updateAnomalyFeedback(MergedAnomalyResultDTO anomaly, AnomalyFeedbackDTO newFeedback) {
Review comment:
Hi Vincent, just a quick question, where is the sibling anomaly updated? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] jihaozh merged pull request #5823: [TE] enchance anomaly api to propagate feedback
Posted by GitBox <gi...@apache.org>.
jihaozh merged pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org