- 
                Notifications
    
You must be signed in to change notification settings  - Fork 25.6k
 
[ML] Removing URL from inference API logs and adding the cause to the REST response #123638
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.
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
[ML] Removing URL from inference API logs and adding the cause to the REST response #123638
Conversation
| l.onFailure( | ||
| createInternalServerError( | ||
| unwrappedException, | ||
| Strings.format("%s. Cause: %s", errorMessage, unwrappedException.getMessage()) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the exception is not an ElasticsearchException we'll grab the cause to return to the API call. We've noticed in the case of apache timeout exceptions it would have been useful to return that information directly to the client instead of only logging it.
| this.model = Objects.requireNonNull(model); | ||
| this.sender = Objects.requireNonNull(sender); | ||
| this.account = new AlibabaCloudSearchAccount(this.model.getSecretSettings().apiKey()); | ||
| this.failedToSendRequestErrorMessage = constructFailedToSendRequestMessage(null, "AlibabaCloud Search completion"); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are to remove the URL parameter.
| 
               | 
          ||
| doAnswer(invocation -> { | ||
| @SuppressWarnings("unchecked") | ||
| ActionListener<InferenceServiceResults> listener = (ActionListener<InferenceServiceResults>) invocation.getArguments()[2]; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we are including the cause I noticed that these tests where retrieving the wrong parameter.
| 
           Pinging @elastic/ml-core (Team:ML)  | 
    
          💔 Backport failed
 You can use sqren/backport to manually backport by running   | 
    
          💚 All backports created successfully
 Questions ?Please refer to the Backport tool documentation  | 
    
(cherry picked from commit 4dcc81f) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIActionCreator.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIEmbeddingsActionTests.java
(cherry picked from commit 4dcc81f) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIActionCreator.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIEmbeddingsActionTests.java
(cherry picked from commit 4dcc81f) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIActionCreator.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIEmbeddingsActionTests.java
(cherry picked from commit 4dcc81f) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIActionCreator.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIEmbeddingsActionTests.java
(cherry picked from commit 4dcc81f) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIActionCreator.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIEmbeddingsActionTests.java
) * Removing url and adding cause REST api response (#123638) (cherry picked from commit 4dcc81f) # Conflicts: # x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIActionCreator.java # x-pack/plugin/inference/src/test/java/org/elasticsearch/xpack/inference/external/action/voyageai/VoyageAIEmbeddingsActionTests.java * Fixing errors
This PR improves the error message that we return in the REST api call when performing inference using the inference API.
It also removes the URL from the message just in case it's getting logged.