Skip to content

HDDS-6247. Avoid logging stack trace for user input problems #3039

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

Merged
adoroszlai merged 3 commits into from
Feb 9, 2022
Merged

HDDS-6247. Avoid logging stack trace for user input problems #3039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3e4bd1
HDDS-6247. Avoid logging stack trace for user input problems
adoroszlai Feb 2, 2022
b3f32d9
Skip error log in headBucket (for NOT_FOUND/ACCESS_DENIED)
adoroszlai Feb 2, 2022
e17a71e
Merge remote-tracking branch 'origin/master' into HDDS-6247
adoroszlai Feb 3, 2022
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
15 changes: 5 additions & 10 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,12 +241,12 @@ public Response put(@PathParam("bucket") String bucketName,
return Response.status(HttpStatus.SC_OK).header("Location", location)
.build();
} catch (OMException exception) {
LOG.error("Error in Create Bucket Request for bucket: {}", bucketName,
exception);
if (exception.getResult() == ResultCodes.INVALID_BUCKET_NAME) {
throw S3ErrorTable.newError(S3ErrorTable.INVALID_BUCKET_NAME,
bucketName);
}
LOG.error("Error in Create Bucket Request for bucket: {}", bucketName,
exception);
throw exception;
}
}
Expand Down Expand Up @@ -291,12 +291,7 @@ public Response listMultipartUploads(
@HEAD
public Response head(@PathParam("bucket") String bucketName)
throws OS3Exception, IOException {
try {
getBucket(bucketName);
} catch (OS3Exception ex) {
LOG.error("Exception occurred in headBucket", ex);
throw ex;
}
getBucket(bucketName);
return Response.ok().build();
}

Expand Down Expand Up @@ -507,15 +502,15 @@ public Response putAcl(String bucketName, HttpHeaders httpHeaders,
volume.addAcl(acl);
}
} catch (OMException exception) {
LOG.error("Error in set ACL Request for bucket: {}", bucketName,
exception);
if (exception.getResult() == ResultCodes.BUCKET_NOT_FOUND) {
throw S3ErrorTable.newError(S3ErrorTable.NO_SUCH_BUCKET,
bucketName);
} else if (exception.getResult() == ResultCodes.PERMISSION_DENIED) {
throw S3ErrorTable.newError(S3ErrorTable
.ACCESS_DENIED, bucketName);
}
LOG.error("Error in set ACL Request for bucket: {}", bucketName,
exception);
throw exception;
}
return Response.status(HttpStatus.SC_OK).build();
Expand Down
37 changes: 17 additions & 20 deletions hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,23 +208,20 @@ public Response put(

return Response.ok().status(HttpStatus.SC_OK)
.build();
} catch (IOException ex) {
Copy link
Member

@ayushtkn ayushtkn Feb 7, 2022

Choose a reason for hiding this comment

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

Why do you want to change IOE to OME? Not logging for known OME makes sense, but now any IOE will get propagated directly without logging. If I catch the intention was to not log the know client issues, like permissions and stuff. But IOE can surface from any reason

Copy link
Contributor Author

@adoroszlai adoroszlai Feb 7, 2022

Choose a reason for hiding this comment

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

Thanks @ayushtkn for the question.

All other methods in ObjectEndpoint only catch OMException and directly propagate IOException.

$ grep -F 'catch (' hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (OMException ex) {
    } catch (UnsupportedEncodingException e) {
    } catch (IllegalArgumentException ex) {
    } catch (ParseException e) {

Uncaught IOException is handled at a different layer, and logged with stack trace:

s3g_1       | 2022-02-07 10:11:02,633 [qtp1498438472-16] WARN server.HttpChannel: handleException /bucket-ozone-test-2711154630/ozone-test-4349237846/putobject/key%3Dvalue/f1 java.io.IOException: TESTING
...
s3g_1       | Caused by: java.io.IOException: TESTING
s3g_1       | 	at org.apache.hadoop.ozone.s3.endpoint.ObjectEndpoint.put(ObjectEndpoint.java:200)
...

Copy link
Member

@ayushtkn ayushtkn Feb 9, 2022

Choose a reason for hiding this comment

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

Makes sense. Should be Ok then

LOG.error("Exception occurred in PutObject", ex);
if (ex instanceof OMException) {
if (((OMException) ex).getResult() == ResultCodes.NOT_A_FILE) {
OS3Exception os3Exception = S3ErrorTable.newError(INVALID_REQUEST,
keyPath);
os3Exception.setErrorMessage("An error occurred (InvalidRequest) " +
"when calling the PutObject/MPU PartUpload operation: " +
OZONE_OM_ENABLE_FILESYSTEM_PATHS + " is enabled Keys are" +
" considered as Unix Paths. Path has Violated FS Semantics " +
"which caused put operation to fail.");
throw os3Exception;
} else if ((((OMException) ex).getResult() ==
ResultCodes.PERMISSION_DENIED)) {
throw S3ErrorTable.newError(S3ErrorTable.ACCESS_DENIED, keyPath);
}
} catch (OMException ex) {
if (ex.getResult() == ResultCodes.NOT_A_FILE) {
OS3Exception os3Exception = S3ErrorTable.newError(INVALID_REQUEST,
keyPath);
os3Exception.setErrorMessage("An error occurred (InvalidRequest) " +
"when calling the PutObject/MPU PartUpload operation: " +
OZONE_OM_ENABLE_FILESYSTEM_PATHS + " is enabled Keys are" +
" considered as Unix Paths. Path has Violated FS Semantics " +
"which caused put operation to fail.");
throw os3Exception;
} else if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
throw S3ErrorTable.newError(S3ErrorTable.ACCESS_DENIED, keyPath);
}
LOG.error("Exception occurred in PutObject", ex);
throw ex;
} finally {
if (output != null) {
Expand Down Expand Up @@ -497,11 +494,11 @@ public Response initializeMultipartUpload(
return Response.status(Status.OK).entity(
multipartUploadInitiateResponse).build();
} catch (OMException ex) {
LOG.error("Error in Initiate Multipart Upload Request for bucket: {}, " +
"key: {}", bucket, key, ex);
if (ex.getResult() == ResultCodes.PERMISSION_DENIED) {
throw S3ErrorTable.newError(S3ErrorTable.ACCESS_DENIED, key);
}
LOG.error("Error in Initiate Multipart Upload Request for bucket: {}, " +
"key: {}", bucket, key, ex);
throw ex;
}
}
Expand Down Expand Up @@ -544,8 +541,6 @@ public Response completeMultipartUpload(@PathParam("bucket") String bucket,
return Response.status(Status.OK).entity(completeMultipartUploadResponse)
.build();
} catch (OMException ex) {
LOG.error("Error in Complete Multipart Upload Request for bucket: {}, " +
", key: {}", bucket, key, ex);
if (ex.getResult() == ResultCodes.INVALID_PART) {
throw S3ErrorTable.newError(S3ErrorTable.INVALID_PART, key);
} else if (ex.getResult() == ResultCodes.INVALID_PART_ORDER) {
Expand All @@ -569,6 +564,8 @@ public Response completeMultipartUpload(@PathParam("bucket") String bucket,
"given KeyName caused failure for MPU");
throw os3Exception;
}
LOG.error("Error in Complete Multipart Upload Request for bucket: {}, " +
", key: {}", bucket, key, ex);
throw ex;
}
}
Expand Down