HDDS-15000. Improve S3 audit log stack traces#10097
Conversation
priyeshkaratha
left a comment
There was a problem hiding this comment.
Thanks @adoroszlai for the patch. Changes overall LGTM. Please check some minor comments.
|
|
||
| public static OS3Exception newError(S3ErrorTable e, String resource) { | ||
| return newError(e, resource, null); | ||
| /** Converts result code of @{code OMException} to {@code S3ErrorTable}, which |
There was a problem hiding this comment.
There is a typo in the Javadoc tag. It should be {@code instead of @{code.
| return new OS3Exception(errorCode, null, resource); | ||
| } | ||
|
|
||
| /** Creates new {@link OS3Exception} for {@link S3ErrorTable}, {@code resource} and {@code cause}. */ |
There was a problem hiding this comment.
The Javadoc comment for this method is inaccurate as it mentions a resource parameter which is not present in this specific overload.
ivandika3
left a comment
There was a problem hiding this comment.
Thanks @adoroszlai for the improvement, left some comments.
ivandika3
left a comment
There was a problem hiding this comment.
Thanks @adoroszlai , LGTM +1.
Russole
left a comment
There was a problem hiding this comment.
Thanks @adoroszlai for working on this. LGTM!
|
Thanks @adoroszlai for the improvement and @priyeshkaratha @Russole for the reviews. |
|
Thanks @ivandika3, @priyeshkaratha, @Russole for the review. |
What changes were proposed in this pull request?
Audit log shows only the last two elements of stack traces. For several failed S3 Gateway requests it is always the same:
due to two levels of
newErrorcalls.This PR changes code to avod chained
newErrorcalls by creatingOS3Exceptionin variousnewErrorimplementations. Related changes to avoid duplication:OS3Exceptioncreation:S3ErrorTable) instead of separate variablesresourcenewErrornewErrorto reducenullarguments in callers.BucketEndpoint#translateExceptiontoS3ErrorTable(renamed totranslateResultCode), add more cases toswitch. ChangeObjectEndpointto rely on result code translation innewError.Related minor fix:
ERRORinstead ofSUCCESS, becauseNOT_FOUNDerror is handled in the outerdeletemethod, nothandleDeleteRequest. This is now moved to let audit log reflect the correct result.https://issues.apache.org/jira/browse/HDDS-15000
How was this patch tested?
Before:
After:
Before:
After:
Before:
After:
https://github.com/adoroszlai/ozone/actions/runs/24665673476