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.

Sign up for GitHub

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

Replace String concatenation with Log4j ParameterizedMessage for readability #943

Merged
merged 11 commits into from
Nov 11, 2024
Merged

Replace String concatenation with Log4j ParameterizedMessage for readability #943

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
713caa1
Replace strings in GetWorkflowTransportAction.java
KirrTap Oct 24, 2024
c03bae8
Merge branch 'opensearch-project:main' into replace-strings
KirrTap Oct 24, 2024
0c4e600
Replace strings in 5 files
KirrTap Oct 27, 2024
d168f5d
Replace strings in 10 files
KirrTap Oct 28, 2024
7e02230
Merge branch 'opensearch-project:main' into replace-strings
KirrTap Nov 3, 2024
7cd4a0e
Replace strings in 40 files
KirrTap Nov 4, 2024
a82fdb5
Merge branch 'opensearch-project:main' into replace-strings
KirrTap Nov 6, 2024
7419752
Changed back strings, which were not be parametrized
KirrTap Nov 6, 2024
dfffc4e
Merge branch 'opensearch-project:main' into replace-strings
KirrTap Nov 7, 2024
5b84049
Update CHANGELOG.md
KirrTap Nov 7, 2024
91fd142
Add EOF line break
dbwiddis Nov 8, 2024
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
104 changes: 81 additions & 23 deletions src/main/java/org/opensearch/flowframework/indices/FlowFrameworkIndicesHandler.java
View file
Open in desktop

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion src/main/java/org/opensearch/flowframework/model/WorkflowNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.ToXContentObject;
Expand Down Expand Up @@ -171,7 +172,10 @@ public static WorkflowNode parse(XContentParser parser) throws IOException {
String configurationsString = ParseUtils.parseArbitraryStringToObjectMapToString(configurationsMap);
userInputs.put(inputFieldName, configurationsString);
} catch (Exception ex) {
String errorMessage = "Failed to parse" + inputFieldName + "map";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to parse {} map",
inputFieldName
).getFormattedMessage();
logger.error(errorMessage, ex);
throw new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST);
}
Expand Down
43 changes: 33 additions & 10 deletions src/main/java/org/opensearch/flowframework/transport/CreateWorkflowTransportAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.opensearch.ExceptionsHelper;
import org.opensearch.action.get.GetRequest;
import org.opensearch.action.search.SearchRequest;
Expand Down Expand Up @@ -201,7 +202,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
try {
validateWorkflows(templateWithUser);
} catch (Exception e) {
String errorMessage = "Workflow validation failed for template " + templateWithUser.name();
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Workflow validation failed for template {}",
templateWithUser.name()
).getFormattedMessage();
logger.error(errorMessage, e);
listener.onFailure(
e instanceof FlowFrameworkException ? e : new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e))
Expand All @@ -218,7 +222,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
flowFrameworkSettings.getMaxWorkflows(),
ActionListener.wrap(max -> {
if (FALSE.equals(max)) {
String errorMessage = "Maximum workflows limit reached: " + flowFrameworkSettings.getMaxWorkflows();
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Maximum workflows limit reached: {}",
flowFrameworkSettings.getMaxWorkflows()
).getFormattedMessage();
logger.error(errorMessage);
FlowFrameworkException ffe = new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST);
listener.onFailure(ffe);
Expand Down Expand Up @@ -307,7 +314,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
}));
}
}, exception -> {
String errorMessage = "Failed to update use case template " + request.getWorkflowId();
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to update use case template {}",
request.getWorkflowId()
).getFormattedMessage();
logger.error(errorMessage, exception);
if (exception instanceof FlowFrameworkException) {
listener.onFailure(exception);
Expand Down Expand Up @@ -350,7 +360,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
ActionListener.wrap(reprovisionResponse -> {
listener.onResponse(new WorkflowResponse(reprovisionResponse.getWorkflowId()));
}, exception -> {
String errorMessage = "Reprovisioning failed for workflow " + workflowId;
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Reprovisioning failed for workflow {}",
workflowId
).getFormattedMessage();
logger.error(errorMessage, exception);
if (exception instanceof FlowFrameworkException) {
listener.onFailure(exception);
Expand Down Expand Up @@ -382,9 +395,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
);
listener.onResponse(new WorkflowResponse(request.getWorkflowId()));
}, exception -> {
String errorMessage = "Failed to update workflow "
+ request.getWorkflowId()
+ " in template index";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to update workflow {} in template index",
request.getWorkflowId()
).getFormattedMessage();
logger.error(errorMessage, exception);
if (exception instanceof FlowFrameworkException) {
listener.onFailure(exception);
Expand All @@ -399,7 +413,10 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
listener.onResponse(new WorkflowResponse(request.getWorkflowId()));
}
}, exception -> {
String errorMessage = "Failed to update use case template " + request.getWorkflowId();
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to update use case template {}",
request.getWorkflowId()
).getFormattedMessage();
logger.error(errorMessage, exception);
if (exception instanceof FlowFrameworkException) {
listener.onFailure(exception);
Expand All @@ -411,12 +428,18 @@ private void createExecute(WorkflowRequest request, User user, ActionListener<Wo
);
}
} else {
String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template ({}) from global context.",
workflowId
).getFormattedMessage();
logger.error(errorMessage);
listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
}
}, exception -> {
String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template ({}) from global context.",
workflowId
).getFormattedMessage();
logger.error(errorMessage, exception);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
}));
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/opensearch/flowframework/transport/DeprovisionWorkflowTransportAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.opensearch.ExceptionsHelper;
import org.opensearch.OpenSearchStatusException;
import org.opensearch.action.support.ActionFilters;
Expand Down Expand Up @@ -166,7 +167,10 @@ private void executeDeprovisionRequest(
)
);
}, exception -> {
String errorMessage = "Failed to get workflow state for workflow " + workflowId;
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to get workflow state for workflow {}",
workflowId
).getFormattedMessage();
logger.error(errorMessage, exception);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
}));
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/org/opensearch/flowframework/transport/GetWorkflowStateTransportAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.opensearch.ExceptionsHelper;
import org.opensearch.action.get.GetRequest;
import org.opensearch.action.support.ActionFilters;
Expand Down Expand Up @@ -96,7 +97,8 @@ protected void doExecute(Task task, GetWorkflowStateRequest request, ActionListe
);

} catch (Exception e) {
String errorMessage = "Failed to get workflow: " + workflowId;
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage("Failed to get workflow: {}", workflowId)
.getFormattedMessage();
logger.error(errorMessage, e);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e)));
}
Expand All @@ -123,7 +125,8 @@ private void executeGetWorkflowStateRequest(
WorkflowState workflowState = WorkflowState.parse(parser);
listener.onResponse(new GetWorkflowStateResponse(workflowState, request.getAll()));
} catch (Exception e) {
String errorMessage = "Failed to parse workflowState: " + r.getId();
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage("Failed to parse workflowState: {}", r.getId())
.getFormattedMessage();
logger.error(errorMessage, e);
listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.BAD_REQUEST));
}
Expand All @@ -134,7 +137,8 @@ private void executeGetWorkflowStateRequest(
if (e instanceof IndexNotFoundException) {
listener.onFailure(new FlowFrameworkException("Fail to find workflow status of " + workflowId, RestStatus.NOT_FOUND));
} else {
String errorMessage = "Failed to get workflow status of: " + workflowId;
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage("Failed to get workflow status of: {}", workflowId)
.getFormattedMessage();
logger.error(errorMessage, e);
listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
}
Expand Down
16 changes: 13 additions & 3 deletions src/main/java/org/opensearch/flowframework/transport/GetWorkflowTransportAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.opensearch.ExceptionsHelper;
import org.opensearch.action.get.GetRequest;
import org.opensearch.action.support.ActionFilters;
Expand Down Expand Up @@ -104,7 +105,10 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<GetW
xContentRegistry
);
} catch (Exception e) {
String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template ({}) from global context.",
workflowId
).getFormattedMessage();
logger.error(errorMessage, e);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e)));
}
Expand Down Expand Up @@ -134,7 +138,10 @@ private void executeGetRequest(
context.restore();

if (!response.isExists()) {
String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template ({}) from global context.",
workflowId
).getFormattedMessage();
logger.error(errorMessage);
listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
} else {
Expand All @@ -144,7 +151,10 @@ private void executeGetRequest(
listener.onResponse(new GetWorkflowResponse(template));
}
}, exception -> {
String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template ({}) from global context.",
workflowId
).getFormattedMessage();
logger.error(errorMessage, exception);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
}));
Expand Down
26 changes: 21 additions & 5 deletions src/main/java/org/opensearch/flowframework/transport/ProvisionWorkflowTransportAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.opensearch.ExceptionsHelper;
import org.opensearch.action.get.GetRequest;
import org.opensearch.action.support.ActionFilters;
Expand Down Expand Up @@ -138,7 +139,10 @@ protected void doExecute(Task task, WorkflowRequest request, ActionListener<Work
xContentRegistry
);
} catch (Exception e) {
String errorMessage = "Failed to retrieve template from global context for workflow " + workflowId;
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template from global context for workflow {}",
workflowId
).getFormattedMessage();
logger.error(errorMessage, e);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(e)));
}
Expand Down Expand Up @@ -169,7 +173,10 @@ private void executeProvisionRequest(
context.restore();

if (!response.isExists()) {
String errorMessage = "Failed to retrieve template (" + workflowId + ") from global context.";
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template ({}) from global context.",
workflowId
).getFormattedMessage();
logger.error(errorMessage);
listener.onFailure(new FlowFrameworkException(errorMessage, RestStatus.NOT_FOUND));
return;
Expand Down Expand Up @@ -212,7 +219,10 @@ private void executeProvisionRequest(
ActionListener.wrap(templateResponse -> {
listener.onResponse(new WorkflowResponse(request.getWorkflowId()));
}, exception -> {
String errorMessage = "Failed to update use case template " + request.getWorkflowId();
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to update use case template {}",
request.getWorkflowId()
).getFormattedMessage();
logger.error(errorMessage, exception);
if (exception instanceof FlowFrameworkException) {
listener.onFailure(exception);
Expand All @@ -224,7 +234,10 @@ private void executeProvisionRequest(
true
);
}, exception -> {
String errorMessage = "Failed to update workflow state: " + workflowId;
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to update workflow state: {}",
workflowId
).getFormattedMessage();
logger.error(errorMessage, exception);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
})
Expand All @@ -244,7 +257,10 @@ private void executeProvisionRequest(
logger.error("Workflow validation failed for workflow {}", workflowId);
listener.onFailure(exception);
} else {
String errorMessage = "Failed to retrieve template from global context for workflow " + workflowId;
String errorMessage = ParameterizedMessageFactory.INSTANCE.newMessage(
"Failed to retrieve template from global context for workflow {}",
workflowId
).getFormattedMessage();
logger.error(errorMessage, exception);
listener.onFailure(new FlowFrameworkException(errorMessage, ExceptionsHelper.status(exception)));
}
Expand Down
Loading