Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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 @@ -70,7 +70,7 @@ public KruizeObject generateResourceRecommendationsForExperiment(String experime
try {
LOGGER.debug(AnalyzerConstants.AutoscalerConstants.InfoMsgs.GENERATING_RECOMMENDATIONS, experimentName);
// generating latest recommendations for experiment
RecommendationEngine recommendationEngine = new RecommendationEngine(experimentName, null, null);
RecommendationEngine recommendationEngine = new RecommendationEngine(experimentName, null, null, null);
int calCount = 0;
String validationMessage = recommendationEngine.validate_local();
if (validationMessage.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.autotune.common.data.ValidationOutputData;
import com.autotune.common.data.result.ExperimentResultData;
import com.autotune.database.service.ExperimentDBService;
import com.autotune.utils.KruizeConstants;
import com.google.gson.annotations.SerializedName;
import jakarta.validation.ConstraintViolation;
import jakarta.validation.Validation;
Expand Down Expand Up @@ -157,6 +158,18 @@ public void validateAndAddExperimentResults(List<UpdateResultsAPIObject> updateR
failedUpdateResultsAPIObjects.add(object);
continue;
}
// log and validate requestId
String requestId = object.getRequest_id();
Copy link
Member

Choose a reason for hiding this comment

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

If requestId scope is only in if-block can we directly check the object.getRequest_id and assign to requestId inside the if-block for a better scoping of the variable?

something like this:

if (null != object.getRequest_id()) {
    String requestId = object.getRequest_id();
    LOGGER.info("request_id : {}", requestId);
    errorMsg = validateRequestId(requestId);
    if (!errorMsg.isEmpty()) {
        errorReasons.add(errorMsg);
        object.setErrors(getErrorMap(errorReasons));
        failedUpdateResultsAPIObjects.add(object);
        continue;
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if (requestId != null) {
LOGGER.info("request_id : {}", requestId);
errorMsg = validateRequestId(requestId);
if (!errorMsg.isEmpty()) {
errorReasons.add(errorMsg);
object.setErrors(getErrorMap(errorReasons));
failedUpdateResultsAPIObjects.add(object);
continue;
}
}
object.setKruizeObject(mainKruizeExperimentMAP.get(object.getExperimentName()));
Set<ConstraintViolation<UpdateResultsAPIObject>> violations = new HashSet<>();
try {
Expand Down Expand Up @@ -241,5 +254,14 @@ public List<UpdateResultsAPIObject> getFailedUpdateResultsAPIObjects() {
return failedUpdateResultsAPIObjects;
}

public static String validateRequestId(String requestId) {
String errorMessage = "";
String pattern = "^[a-zA-Z0-9]{" + KruizeConstants.KRUIZE_CONFIG_DEFAULT_VALUE.REQUEST_ID_LENGTH + "}$";
if (!requestId.matches(pattern)) {
errorMessage = KruizeConstants.ErrorMsgs.APIErrorMsgs.INVALID_REQUEST_ID;
}
return errorMessage;
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,13 @@ public void validate(List<KruizeObject> kruizeExptList) {
}
}
}

// validate request_id, if present
if (kruizeObject.getRequestId() != null) {
errorMsg = ExperimentInitiator.validateRequestId(kruizeObject.getRequestId());
validationOutputData.setErrorCode(HttpServletResponse.SC_BAD_REQUEST);
}

} else {
errorMsg = errorMsg.concat(String.format(AnalyzerErrorConstants.AutotuneObjectErrors.DUPLICATE_EXPERIMENT)).concat(expName);
validationOutputData.setErrorCode(HttpServletResponse.SC_CONFLICT);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ public final class KruizeObject implements ExperimentTypeAware {
private List<K8sObject> kubernetes_objects;
private Map<String, Terms> terms;
private transient String bulkJobId;
private transient String requestId;

public KruizeObject(String experimentName,
String clusterName,
Expand Down Expand Up @@ -395,6 +396,13 @@ public void setMetadataProfile(String metadataProfile) {
this.metadataProfile = metadataProfile;
}

public String getRequestId() {
return requestId;
}

public void setRequestId(String requestId) {
this.requestId = requestId;
}

@Override
public String toString() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.autotune.analyzer.exceptions.FetchMetricsError;
import com.autotune.analyzer.exceptions.InvalidModelException;
import com.autotune.analyzer.exceptions.InvalidTermException;
import com.autotune.analyzer.experiment.ExperimentInitiator;
import com.autotune.analyzer.kruizeObject.KruizeObject;
import com.autotune.analyzer.kruizeObject.ModelSettings;
import com.autotune.analyzer.kruizeObject.RecommendationSettings;
Expand Down Expand Up @@ -73,13 +74,14 @@ public class RecommendationEngine {
private Timestamp interval_end_time;
private List<String> modelNames;
private Map<String, RecommendationTunables> modelTunable;
private String requestId;


public RecommendationEngine(String experimentName, String intervalEndTimeStr, String intervalStartTimeStr) {
public RecommendationEngine(String experimentName, String intervalEndTimeStr, String intervalStartTimeStr, String requestId) {
this.experimentName = experimentName;
this.intervalEndTimeStr = intervalEndTimeStr;
this.intervalStartTimeStr = intervalStartTimeStr;

this.requestId = requestId;
}

private static int getNumPods(Map<Timestamp, IntervalResults> filteredResultsMap) {
Expand Down Expand Up @@ -294,6 +296,11 @@ public String validate() {
// Check if interval_start_time is provided
// TODO: to be considered in future

// validate request_id, if present
if (requestId != null) {
validationFailureMsg = ExperimentInitiator.validateRequestId(requestId);
}

return validationFailureMsg;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ public static KruizeObject convertCreateExperimentAPIObjToKruizeObject(CreateExp
// Validation already done, and it is getting loaded back from db
kruizeObject.setValidation_data(createExperimentAPIObject.getValidationData());
}
// add request_id
kruizeObject.setRequestId(createExperimentAPIObject.getRequest_id());
} catch (Exception e) {
LOGGER.error("failed to convert CreateExperimentAPIObj To KruizeObject due to {} ", e.getMessage());
LOGGER.debug(createExperimentAPIObject.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public class CreateExperimentAPIObject extends BaseSO implements ExperimentTypeA
private AnalyzerConstants.ExperimentStatus status;
private String experiment_id; // this id is UUID and getting set at createExperiment API
private ValidationOutputData validationData; // This object indicates if this API object is valid or invalid
private String request_id; // this gets logged to uniquely identify each request

public CreateExperimentAPIObject() {
}
Expand Down Expand Up @@ -170,6 +171,10 @@ public void setExperimentType(AnalyzerConstants.ExperimentType experimentType) {
this.experimentType = experimentType;
}

public String getRequest_id() {
Copy link
Member

Choose a reason for hiding this comment

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

Can we refactor -> rename the method name to getRequestId to keep it consistent across the code base.

[In Kruize Object we are maintaining it as getRequestId so just thought it would be better to use Camel Case rather than Snake Case]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

return request_id;
}

@Override
public String toString() {
return "CreateExperimentAPIObject{" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public class UpdateResultsAPIObject extends BaseSO {
private List<KruizeResponse> errors;

private KruizeObject kruizeObject;
private String request_id; // this gets logged to uniquely identify each request

public Timestamp getStartTimestamp() {
return startTimestamp;
Expand Down Expand Up @@ -109,6 +110,10 @@ public interface EvaluateRemainingConstraints {
public interface EvaluatePerformanceProfileConstraints {
}

public String getRequest_id() {
Copy link
Member

Choose a reason for hiding this comment

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

Can we refactor -> rename the method name to getRequestId to keep it consistent across the code base.

[In Kruize Object we are maintaining it as getRequestId so just thought it would be better to use Camel Case rather than Snake Case]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Refactored now.

return request_id;
}

@GroupSequence({UpdateResultsAPIObject.class, InitialValidation.class, EvaluatePerformanceProfileConstraints.class, EvaluateRemainingConstraints.class})
public interface FullValidationSequence {
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
KruizeObject kruizeObject = Converters.KruizeObjectConverters.convertCreateExperimentAPIObjToKruizeObject(createExperimentAPIObject);
if (null != kruizeObject)
kruizeExpList.add(kruizeObject);
// log request_id if available
if (createExperimentAPIObject.getRequest_id() != null) {
LOGGER.info("request_id : {}", createExperimentAPIObject.getRequest_id());
}
}
new ExperimentInitiator().validateAndAddNewExperiments(mKruizeExperimentMap, kruizeExpList);
//TODO: UX needs to be modified - Handle response for the multiple objects
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
Timestamp interval_end_time, interval_start_time;

// create recommendation engine object
RecommendationEngine recommendationEngine = new RecommendationEngine(experiment_name, intervalEndTimeStr, intervalStartTimeStr);
RecommendationEngine recommendationEngine = new RecommendationEngine(experiment_name, intervalEndTimeStr, intervalStartTimeStr, null);
// validate and create KruizeObject if successful
String validationMessage = recommendationEngine.validate_local();
if (validationMessage.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
// Get the values from the request parameters
String experiment_name = request.getParameter(KruizeConstants.JSONKeys.EXPERIMENT_NAME);
String intervalEndTimeStr = request.getParameter(KruizeConstants.JSONKeys.INTERVAL_END_TIME);
String requestId = request.getParameter(KruizeConstants.JSONKeys.REQUEST_ID); // this gets logged to uniquely identify each request

String intervalStartTimeStr = request.getParameter(KruizeConstants.JSONKeys.INTERVAL_START_TIME);
Timestamp interval_end_time;
Expand All @@ -100,7 +101,7 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
LOGGER.info(String.format(KruizeConstants.APIMessages.UPDATE_RECOMMENDATIONS_INPUT_PARAMS, experiment_name, intervalStartTimeStr, intervalEndTimeStr));
try {
// create recommendation engine object
RecommendationEngine recommendationEngine = new RecommendationEngine(experiment_name, intervalEndTimeStr, intervalStartTimeStr);
RecommendationEngine recommendationEngine = new RecommendationEngine(experiment_name, intervalEndTimeStr, intervalStartTimeStr, requestId);
// validate and create KruizeObject if successful
String validationMessage = recommendationEngine.validate();
if (validationMessage.isEmpty()) {
Expand Down Expand Up @@ -135,6 +136,10 @@ protected void doPost(HttpServletRequest request, HttpServletResponse response)
MetricsConfig.timerUpdateRecomendations = MetricsConfig.timerBUpdateRecommendations.tag(KruizeConstants.DataSourceConstants.DataSourceQueryJSONKeys.STATUS, statusValue).register(MetricsConfig.meterRegistry());
timerBUpdateRecommendations.stop(MetricsConfig.timerUpdateRecomendations);
}
// log request_id if available
if (requestId != null) {
LOGGER.info("request_id : {}", requestId);
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/autotune/utils/KruizeConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ public static final class JSONKeys {
public static final String CONTAINER_IMAGE_NAME = "container_image_name";
public static final String RECOMMENDATION_SETTINGS = "recommendation_settings";
public static final String INTERVAL_START_TIME = "interval_start_time";
public static final String REQUEST_ID = "request_id";

public static final String CALCULATED_START_TIME = "calculated_start_time";
public static final String INTERVAL_END_TIME = "interval_end_time";
Expand Down Expand Up @@ -646,6 +647,7 @@ private ErrorMsgs() {
public static class APIErrorMsgs {
private APIErrorMsgs() {
}
public static final String INVALID_REQUEST_ID = "Invalid requestId format. Should be a "+KRUIZE_CONFIG_DEFAULT_VALUE.REQUEST_ID_LENGTH+"-character alphanumeric";

public static class ListDeploymentsInNamespace {
public static final String INVALID_NAMESPACE = "Given Namespace is invalid";
Expand Down Expand Up @@ -840,6 +842,7 @@ private RecommendationDurationRanges() {

public static final class KRUIZE_CONFIG_DEFAULT_VALUE {
public static final int DELETE_PARTITION_THRESHOLD_IN_DAYS = 16;
public static final int REQUEST_ID_LENGTH = 32;
}

public static final class KRUIZE_RECOMMENDATION_METRICS {
Expand Down