clean logging and fix typos

Signed-off-by: HenryL27 <[email protected]>
HenryL27 · Aug 30, 2023 · fa8a3f6 · fa8a3f6
1 parent f0b4d55
commit fa8a3f6
Show file tree

Hide file tree

Showing 11 changed files with 38 additions and 38 deletions.
diff --git a/...ersational-memory/src/main/java/org/opensearch/ml/memory/ConversationalMemoryHandler.java b/...ersational-memory/src/main/java/org/opensearch/ml/memory/ConversationalMemoryHandler.java
@@ -63,7 +63,7 @@ public interface ConversationalMemoryHandler {
  * @param promptTemplate the prompt template used for this interaction
  * @param response the Gen AI response for this interaction
  * @param origin the name of the GenAI agent in this interaction
- * @param additionalInfo additional inofrmation used in constructing the LLM prompt
+ * @param additionalInfo additional information used in constructing the LLM prompt
  * @param listener gets the ID of the new interaction
  */
  public void createInteraction(
@@ -83,16 +83,16 @@ public void createInteraction(
  * @param promptTemplate the prompt template used in this interaction
  * @param response the Gen AI response for this interaction
  * @param origin the name of the GenAI agent in this interaction
- * @param additionalInfo additional inofrmation used in constructing the LLM prompt
+ * @param additionalInfo additional information used in constructing the LLM prompt
  * @return ActionFuture for the interactionId of the new interaction
  */
  public ActionFuture<String> createInteraction(
  String conversationId,
  String input,
  String promptTemplate,
  String response,
- String additionalInfo,
- String metadata
+ String origin,
+ String additionalInfo
  );
 
  /**

diff --git a/.../java/org/opensearch/ml/memory/action/conversation/CreateConversationTransportAction.java b/.../java/org/opensearch/ml/memory/action/conversation/CreateConversationTransportAction.java
@@ -42,7 +42,7 @@ public class CreateConversationTransportAction extends HandledTransportAction<Cr
  /**
  * Constructor
  * @param transportService for inter-node communications
- * @param actionFilters not sure what this is for tbh
+ * @param actionFilters for filtering actions
  * @param cmHandler Handler for conversational memory operations
  * @param client OS Client for dealing with OS
  */

diff --git a/...src/main/java/org/opensearch/ml/memory/action/conversation/CreateInteractionResponse.java b/...src/main/java/org/opensearch/ml/memory/action/conversation/CreateInteractionResponse.java
@@ -36,7 +36,7 @@ public class CreateInteractionResponse extends ActionResponse implements ToXCont
  private String interactionId;
 
  /**
- * Convtructor
+ * Constructor
  * @param in input stream to create this from
  * @throws IOException if something breaks
  */

diff --git a/...n/java/org/opensearch/ml/memory/action/conversation/CreateInteractionTransportAction.java b/...n/java/org/opensearch/ml/memory/action/conversation/CreateInteractionTransportAction.java
@@ -42,7 +42,7 @@ public class CreateInteractionTransportAction extends HandledTransportAction<Cre
  /**
  * Constructor
  * @param transportService for doing intra-cluster communication
- * @param actionFilters not sure what this is for
+ * @param actionFilters for filtering actions
  * @param cmHandler handler for conversational memory
  * @param client client for general opensearch ops
  */
@@ -74,7 +74,7 @@ protected void doExecute(Task task, CreateInteractionRequest request, ActionList
  });
  cmHandler.createInteraction(cid, inp, prompt, rsp, ogn, additionalInfo, al);
  } catch (Exception e) {
- log.error(e.toString());
+ log.error("Failed to create interaction for conversation " + cid, e);
  actionListener.onFailure(e);
  }
  }

diff --git a/.../java/org/opensearch/ml/memory/action/conversation/DeleteConversationTransportAction.java b/.../java/org/opensearch/ml/memory/action/conversation/DeleteConversationTransportAction.java
@@ -42,7 +42,7 @@ public class DeleteConversationTransportAction extends HandledTransportAction<De
  /**
  * Constructor
  * @param transportService for inter-node communications
- * @param actionFilters not sure what this is for tbh
+ * @param actionFilters for filtering actions
  * @param cmHandler Handler for conversational memory operations
  * @param client OS Client for dealing with OS
  */
@@ -69,7 +69,7 @@ public void doExecute(Task task, DeleteConversationRequest request, ActionListen
  }, e -> { internalListener.onFailure(e); });
  cmHandler.deleteConversation(conversationId, al);
  } catch (Exception e) {
- log.error(e.toString());
+ log.error("Failed to delete conversation " + conversationId, e);
  listener.onFailure(e);
  }
  }

diff --git a/...in/java/org/opensearch/ml/memory/action/conversation/GetConversationsTransportAction.java b/...in/java/org/opensearch/ml/memory/action/conversation/GetConversationsTransportAction.java
@@ -45,7 +45,7 @@ public class GetConversationsTransportAction extends HandledTransportAction<GetC
  /**
  * Constructor
  * @param transportService for inter-node communications
- * @param actionFilters not sure what this is for tbh
+ * @param actionFilters for filtering actions
  * @param cmHandler Handler for conversational memory operations
  * @param client OS Client for dealing with OS
  */
@@ -71,12 +71,12 @@ public void doExecute(Task task, GetConversationsRequest request, ActionListener
  internalListener
  .onResponse(new GetConversationsResponse(conversations, from + maxResults, conversations.size() == maxResults));
  }, e -> {
- log.error(e.toString());
+ log.error("Failed to get conversations", e);
  internalListener.onFailure(e);
  });
  cmHandler.getConversations(from, maxResults, al);
  } catch (Exception e) {
- log.error(e.toString());
+ log.error("Failed to get conversations", e);
  actionListener.onFailure(e);
  }
  }

diff --git a/...ain/java/org/opensearch/ml/memory/action/conversation/GetInteractionsTransportAction.java b/...ain/java/org/opensearch/ml/memory/action/conversation/GetInteractionsTransportAction.java
@@ -45,7 +45,7 @@ public class GetInteractionsTransportAction extends HandledTransportAction<GetIn
  /**
  * Constructor
  * @param transportService for inter-node communications
- * @param actionFilters not sure what this is for tbh
+ * @param actionFilters for filtering actions
  * @param cmHandler Handler for conversational memory operations
  * @param client OS Client for dealing with OS
  */
@@ -73,7 +73,7 @@ public void doExecute(Task task, GetInteractionsRequest request, ActionListener<
  }, e -> { internalListener.onFailure(e); });
  cmHandler.getInteractions(request.getConversationId(), from, maxResults, al);
  } catch (Exception e) {
- log.error(e.toString());
+ log.error("Failed to get interactions for conversation " + request.getConversationId(), e);
  actionListener.onFailure(e);
  }
 

diff --git a/...ersational-memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java b/...ersational-memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java
@@ -89,7 +89,7 @@ public void initConversationMetaIndexIfAbsent(ActionListener<Boolean> listener)
  || (e instanceof OpenSearchWrapperException && e.getCause() instanceof ResourceAlreadyExistsException)) {
  internalListener.onResponse(true);
  } else {
- log.error("failed to create index [" + indexName + "]");
+ log.error("failed to create index [" + indexName + "]", e);
  internalListener.onFailure(e);
  }
  });
@@ -99,7 +99,7 @@ public void initConversationMetaIndexIfAbsent(ActionListener<Boolean> listener)
  || (e instanceof OpenSearchWrapperException && e.getCause() instanceof ResourceAlreadyExistsException)) {
  listener.onResponse(true);
  } else {
- log.error("failed to create index [" + indexName + "]");
+ log.error("failed to create index [" + indexName + "]", e);
  listener.onFailure(e);
  }
  }
@@ -139,12 +139,12 @@ public void createConversation(String name, ActionListener<String> listener) {
  internalListener.onFailure(new IOException("failed to create conversation"));
  }
  }, e -> {
- log.error("failed to create conversation", e);
+ log.error("Failed to create conversation", e);
  internalListener.onFailure(e);
  });
  client.index(request, al);
  } catch (Exception e) {
- log.error(e.toString());
+ log.error("Failed to create conversation", e);
  listener.onFailure(e);
  }
  } else {
@@ -190,18 +190,18 @@ public void getConversations(int from, int maxResults, ActionListener<List<Conve
  }
  internalListener.onResponse(result);
  }, e -> {
- log.error("failed to list conversations", e);
+ log.error("Failed to retrieve conversations", e);
  internalListener.onFailure(e);
  });
  client
  .admin()
  .indices()
  .refresh(Requests.refreshRequest(indexName), ActionListener.wrap(refreshResponse -> { client.search(request, al); }, e -> {
- log.error("failed during refresh", e);
+ log.error("Failed to retrieve conversations during refresh", e);
  internalListener.onFailure(e);
  }));
  } catch (Exception e) {
- log.error("failed during list conversations", e);
+ log.error("Failed to retrieve conversations", e);
  listener.onFailure(e);
  }
  }
@@ -237,7 +237,7 @@ public void deleteConversation(String conversationId, ActionListener<Boolean> li
  internalListener.onResponse(false);
  }
  }, e -> {
- log.error("failure deleting conversation " + conversationId, e);
+ log.error("Failure deleting conversation " + conversationId, e);
  internalListener.onFailure(e);
  });
  this.checkAccess(conversationId, ActionListener.wrap(access -> {
@@ -253,7 +253,7 @@ public void deleteConversation(String conversationId, ActionListener<Boolean> li
  }
  }, e -> { internalListener.onFailure(e); }));
  } catch (Exception e) {
- log.error("failed deleting conversation with id=" + conversationId, e);
+ log.error("Failed deleting conversation with id=" + conversationId, e);
  listener.onFailure(e);
  }
  }

diff --git a/conversational-memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java b/conversational-memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java
@@ -89,7 +89,7 @@ public void initInteractionsIndexIfAbsent(ActionListener<Boolean> listener) {
  || (e instanceof OpenSearchWrapperException && e.getCause() instanceof ResourceAlreadyExistsException)) {
  internalListener.onResponse(true);
  } else {
- log.error("failed to create index [" + indexName + "]");
+ log.error("Failed to create index [" + indexName + "]", e);
  internalListener.onFailure(e);
  }
  });
@@ -99,7 +99,7 @@ public void initInteractionsIndexIfAbsent(ActionListener<Boolean> listener) {
  || (e instanceof OpenSearchWrapperException && e.getCause() instanceof ResourceAlreadyExistsException)) {
  listener.onResponse(true);
  } else {
- log.error("failed to create index [" + indexName + "]");
+ log.error("Failed to create index [" + indexName + "]", e);
  listener.onFailure(e);
  }
  }
@@ -115,7 +115,7 @@ public void initInteractionsIndexIfAbsent(ActionListener<Boolean> listener) {
  * @param promptTemplate the prompt template used for this interaction
  * @param response the GenAI response for this interaction
  * @param origin the origin of the response for this interaction
- * @param metadata additional information used for constructing the LLM prompt
+ * @param additionalInfo additional information used for constructing the LLM prompt
  * @param timestamp when this interaction happened
  * @param listener gets the id of the newly created interaction record
  */
@@ -125,7 +125,7 @@ public void createInteraction(
  String promptTemplate,
  String response,
  String origin,
- String metadata,
+ String additionalInfo,
  Instant timestamp,
  ActionListener<String> listener
  ) {
@@ -147,7 +147,7 @@ public void createInteraction(
  ConversationalIndexConstants.INTERACTIONS_RESPONSE_FIELD,
  response,
  ConversationalIndexConstants.INTERACTIONS_ADDITIONAL_INFO_FIELD,
- metadata,
+ additionalInfo,
  ConversationalIndexConstants.INTERACTIONS_CREATE_TIME_FIELD,
  timestamp
  );
@@ -157,7 +157,7 @@ public void createInteraction(
  if (resp.status() == RestStatus.CREATED) {
  internalListener.onResponse(resp.getId());
  } else {
- internalListener.onFailure(new IOException("failed to create conversation"));
+ internalListener.onFailure(new IOException("Failed to create interaction"));
  }
  }, e -> { internalListener.onFailure(e); });
  client.index(request, al);
@@ -188,7 +188,7 @@ public void createInteraction(
  * @param promptTemplate the prompt template used for this interaction
  * @param response the GenAI response for this interaction
  * @param origin the name of the GenAI agent this interaction belongs to
- * @param metadata additional information used to construct the LLM prompt
+ * @param additionalInfo additional information used to construct the LLM prompt
  * @param listener gets the id of the newly created interaction record
  */
  public void createInteraction(
@@ -197,10 +197,10 @@ public void createInteraction(
  String promptTemplate,
  String response,
  String origin,
- String metadata,
+ String additionalInfo,
  ActionListener<String> listener
  ) {
- createInteraction(conversationId, input, promptTemplate, response, origin, metadata, Instant.now(), listener);
+ createInteraction(conversationId, input, promptTemplate, response, origin, additionalInfo, Instant.now(), listener);
  }
 
  /**

diff --git a/...y/src/main/java/org/opensearch/ml/memory/index/OpenSearchConversationalMemoryHandler.java b/...y/src/main/java/org/opensearch/ml/memory/index/OpenSearchConversationalMemoryHandler.java
@@ -102,7 +102,7 @@ public ActionFuture<String> createConversation(String name) {
  * @param promptTemplate the prompt template used for this interaction
  * @param response the Gen AI response for this interaction
  * @param origin the name of the GenAI agent in this interaction
- * @param additionalInfo additional inofrmation used in constructing the LLM prompt
+ * @param additionalInfo additional information used in constructing the LLM prompt
  * @param listener gets the ID of the new interaction
  */
  public void createInteraction(
@@ -125,7 +125,7 @@ public void createInteraction(
  * @param promptTemplate the prompt template used in this interaction
  * @param response the Gen AI response for this interaction
  * @param origin the name of the GenAI agent in this interaction
- * @param additionalInfo additional inofrmation used in constructing the LLM prompt
+ * @param additionalInfo additional information used in constructing the LLM prompt
  * @return ActionFuture for the interactionId of the new interaction
  */
  public ActionFuture<String> createInteraction(
@@ -176,7 +176,7 @@ public ActionFuture<String> createInteraction(InteractionBuilder builder) {
  /**
  * Get the interactions associate with this conversation, sorted by recency
  * @param conversationId the conversation whose interactions to get
- * @param from where to start listiing from
+ * @param from where to start listing from
  * @param maxResults how many interactions to get
  * @param listener gets the list of interactions in this conversation, sorted by recency
  */
@@ -187,7 +187,7 @@ public void getInteractions(String conversationId, int from, int maxResults, Act
  /**
  * Get the interactions associate with this conversation, sorted by recency
  * @param conversationId the conversation whose interactions to get
- * @param from where to start listiing from
+ * @param from where to start listing from
  * @param maxResults how many interactions to get
  * @return ActionFuture the list of interactions in this conversation, sorted by recency
  */

diff --git a/...rsational-memory/src/test/java/org/opensearch/ml/memory/index/InteractionsIndexTests.java b/...rsational-memory/src/test/java/org/opensearch/ml/memory/index/InteractionsIndexTests.java
@@ -260,7 +260,7 @@ public void testCreate_BadRestStatus_ThenFail() {
  interactionsIndex.createInteraction("cid", "inp", "pt", "rsp", "ogn", "meta", createInteractionListener);
  ArgumentCaptor<Exception> argCaptor = ArgumentCaptor.forClass(Exception.class);
  verify(createInteractionListener, times(1)).onFailure(argCaptor.capture());
- assert (argCaptor.getValue().getMessage().equals("failed to create conversation"));
+ assert (argCaptor.getValue().getMessage().equals("Failed to create interaction"));
  }
 
  public void testCreate_InternalFailure_ThenFail() {