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.

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] Don't log errors for problems caused by shape of input data #100996

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

droberts195
Copy link
Contributor

Logging errors in response to user input can cause enormous volumes of output, which then obscures errors caused by bugs in the software.

Logging errors in response to user input can cause enormous volumes
of output, which then obscures errors caused by bugs in the software.
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@droberts195
Copy link
Contributor Author

Jenkins run elasticsearch-ci/part-2

@droberts195
Copy link
Contributor Author

Jenkins run elasticsearch-ci/bwc

Copy link
Member

@maxhniebergall maxhniebergall left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +116 to +117
if (e.status().getStatus() >= RestStatus.INTERNAL_SERVER_ERROR.getStatus()) {
logger.error(() -> "[" + getDeploymentId() + "] internal server error running inference", e);
Copy link
Member

Choose a reason for hiding this comment

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

Since we are logging this error for all rest statuses >=500, perhaps it would be best to remove the word "internal" from the logging message, as "Internal Server Error" is only the 500 code itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The other 5xx error codes still relate to things that are internal to the server, just more specific than 500.

These error logs are really for our benefit rather than the end user's, and one important thing about them is that we can easily find which line of code logged them when we see them in the logs. When someone sees one of these in a log file their next question will be, "What could have caused that?" rather than, "Are the exact words used in the message pedantically correct?"

Copy link
Member

@davidkyle davidkyle left a comment

Choose a reason for hiding this comment

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

LGTM

@droberts195 droberts195 merged commit dffd842 into elastic:main Oct 18, 2023
12 checks passed
@droberts195 droberts195 deleted the reduce_error_logging branch October 18, 2023 08:34
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.11

droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Oct 18, 2023
…stic#100996)

Logging errors in response to user input can cause enormous volumes
of output, which then obscures errors caused by bugs in the software.
elasticsearchmachine pushed a commit that referenced this pull request Oct 18, 2023
…0996) (#101036)

Logging errors in response to user input can cause enormous volumes
of output, which then obscures errors caused by bugs in the software.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning >non-issue Team:ML Meta label for the ML team v8.11.1 v8.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants