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

Report S3 operation name of specific request that failed. #377

Merged
merged 14 commits into from
Nov 22, 2023
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
20 changes: 19 additions & 1 deletion include/aws/s3/private/s3_default_meta_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ struct aws_s3_meta_request_default {

size_t content_length;

/* Actual type for the single request (may be AWS_S3_REQUEST_TYPE_UNKNOWN) */
enum aws_s3_request_type request_type;

/* S3 operation name for the single request (NULL if unknown) */
struct aws_string *operation_name;

/* Members to only be used when the mutex in the base type is locked. */
struct {
int cached_response_status;
Expand All @@ -26,10 +32,22 @@ struct aws_s3_meta_request_default {
} synced_data;
};

/* Creates a new default meta request. This will send the request as is and pass back the response. */
/* Creates a new default meta request. This will send the request as is and pass back the response.
*
* Sometimes, a default meta-request is used for other aws_s3_meta_request_types.
* For example, if the request is simple (e.g. single part upload or download),
* or if there's a header that's too complex to deal with.
* In these cases, request_type and operation_name_override should be set.
*
* But if the user literally asked for a AWS_S3_META_REQUEST_TYPE_DEFAULT,
* the request_type should be AWS_S3_META_REQUEST_TYPE_DEFAULT and
* operation_name_override should be NULL (options->operation_name will be used
* if it is set).
*/
struct aws_s3_meta_request *aws_s3_meta_request_default_new(
struct aws_allocator *allocator,
struct aws_s3_client *client,
enum aws_s3_request_type request_type,
uint64_t content_length,
bool should_compute_content_md5,
const struct aws_s3_meta_request_options *options);
Expand Down
3 changes: 0 additions & 3 deletions include/aws/s3/private/s3_meta_request_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,6 @@ struct aws_s3_meta_request_vtable {

/* Pause the given request */
int (*pause)(struct aws_s3_meta_request *meta_request, struct aws_s3_meta_request_resume_token **resume_token);

/* Get the type of the aws_s3_request */
int (*get_request_type)(const struct aws_s3_request *request);
};

/**
Expand Down
23 changes: 20 additions & 3 deletions include/aws/s3/private/s3_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ struct aws_s3_request_metrics {
struct aws_string *host_address;
/* The the request ID header value. */
struct aws_string *request_id;
/* S3 operation name for the request (NULL if unknown) */
struct aws_string *operation_name;
/* The type of request made */
enum aws_s3_request_type request_type;
} req_resp_info_metrics;
Expand Down Expand Up @@ -155,9 +157,18 @@ struct aws_s3_request {
* by the derived type. Request tags do not necessarily map 1:1 with actual S3 API requests. (For example, they can
* be more contextual, like "first part" instead of just "part".) */

/* TODO: this should be a union type to make it clear that this could be one of two enums for puts, and gets. */
/* TODO: Eliminate the concept of "request tag" and just use request_type.
* It's confusing having 2 concepts that are so similar.
* There's only 1 case where 2 tags used the same type,
* we can use some other bool/flag to differentiate this 1 case. */
int request_tag;

/* Actual S3 type for the single request (may be AWS_S3_REQUEST_TYPE_UNKNOWN) */
enum aws_s3_request_type request_type;

/* S3 operation name for the single request (e.g. "CompleteMultipartUpload") (NULL if unknown) */
struct aws_string *operation_name;

/* Members of this structure will be repopulated each time the request is sent. If the request fails, and needs to
* be retried, then the members of this structure will be cleaned up and re-populated on the next send.
*/
Expand Down Expand Up @@ -219,11 +230,16 @@ struct aws_s3_request {

AWS_EXTERN_C_BEGIN

/* Create a new s3 request structure with the given options. */
/* Create a new s3 request structure with the given options.
* @param operation_name Official S3 operation name (e.g. "CreateMultipartUpload").
* Pass an empty string if unknown.
* The string's memory must outlive the request (static string, or live on meta_request)
*/
AWS_S3_API
struct aws_s3_request *aws_s3_request_new(
struct aws_s3_meta_request *meta_request,
int request_tag,
enum aws_s3_request_type request_type,
uint32_t part_number,
uint32_t flags);

Expand All @@ -245,7 +261,8 @@ struct aws_s3_request *aws_s3_request_release(struct aws_s3_request *request);
AWS_S3_API
struct aws_s3_request_metrics *aws_s3_request_metrics_new(
struct aws_allocator *allocator,
struct aws_http_message *message);
const struct aws_s3_request *request,
const struct aws_http_message *message);

AWS_EXTERN_C_END

Expand Down
71 changes: 65 additions & 6 deletions include/aws/s3/s3_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,21 @@ enum aws_s3_meta_request_type {
};

/**
* The type of S3 request made. Used by metrics.
* The type of a single S3 HTTP request. Used by metrics.
* A meta-request can make multiple S3 HTTP requests under the hood.
*
* For example, AWS_S3_META_REQUEST_TYPE_PUT_OBJECT for a large file will
* do multipart upload, resulting in 3+ HTTP requests:
* AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD, one or more AWS_S3_REQUEST_TYPE_UPLOAD_PART,
* and finally AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD.
*
* aws_s3_request_type_operation_name() returns the S3 operation name
* for types that map (e.g. AWS_S3_REQUEST_TYPE_HEAD_OBJECT -> "HeadObject"),
* or empty string for types that don't map (e.g. AWS_S3_REQUEST_TYPE_UNKNOWN -> "").
*/
enum aws_s3_request_type {
/* Same as the original HTTP request passed to aws_s3_meta_request_options */
AWS_S3_REQUEST_TYPE_DEFAULT,
/* The actual type of the single S3 HTTP request is unknown */
AWS_S3_REQUEST_TYPE_UNKNOWN,

/* S3 APIs */
AWS_S3_REQUEST_TYPE_HEAD_OBJECT,
Expand All @@ -100,8 +110,14 @@ enum aws_s3_request_type {
AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD,
AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD,
AWS_S3_REQUEST_TYPE_UPLOAD_PART_COPY,
AWS_S3_REQUEST_TYPE_COPY_OBJECT,
AWS_S3_REQUEST_TYPE_PUT_OBJECT,

/* Max enum value */
AWS_S3_REQUEST_TYPE_MAX,

/** @deprecated Use AWS_S3_REQUEST_TYPE_UNKNOWN if the actual S3 HTTP request type is unknown */
AWS_S3_REQUEST_TYPE_DEFAULT = AWS_S3_REQUEST_TYPE_UNKNOWN,
};

/**
Expand Down Expand Up @@ -481,6 +497,15 @@ struct aws_s3_meta_request_options {
/* The type of meta request we will be trying to accelerate. */
enum aws_s3_meta_request_type type;

/**
* Optional.
* The S3 operation name (e.g. "CreateBucket").
* You should only set this when type is AWS_S3_META_REQUEST_TYPE_DEFAULT;
graebm marked this conversation as resolved.
Show resolved Hide resolved
* it is automatically populated for other meta-request types.
* This name is used to fill out details in metrics and error reports.
*/
struct aws_byte_cursor operation_name;
graebm marked this conversation as resolved.
Show resolved Hide resolved

/* Signing options to be used for each request created for this meta request. If NULL, options in the client will
* be used. If not NULL, these options will override the client options. */
const struct aws_signing_config_aws *signing_config;
Expand Down Expand Up @@ -602,12 +627,23 @@ struct aws_s3_meta_request_options {
*/
struct aws_s3_meta_request_result {

/* HTTP Headers for the failed request that triggered finish of the meta request. NULL if no request failed. */
/* If meta request failed due to an HTTP error response from S3, these are the headers.
* NULL if meta request failed for another reason. */
struct aws_http_headers *error_response_headers;

/* Response body for the failed request that triggered finishing of the meta request. NULL if no request failed.*/
/* If meta request failed due to an HTTP error response from S3, this the body.
* NULL if meta request failed for another reason, or if the response had no body (such as a HEAD response). */
struct aws_byte_buf *error_response_body;

/* If meta request failed due to an HTTP error response from S3,
* this is the name of the S3 operation it was responding to.
* For example, if a AWS_S3_META_REQUEST_TYPE_PUT_OBJECT fails this could be
* "PutObject, "CreateMultipartUpload", "UploadPart", "CompleteMultipartUpload", or others.
* For AWS_S3_META_REQUEST_TYPE_DEFAULT, this is the same value passed to
* aws_s3_meta_request_options.operation_name.
* NULL if the meta request failed for another reason, or the operation name is not known. */
struct aws_string *error_response_operation_name;

/* Response status of the failed request or of the entire meta request. */
int response_status;

Expand Down Expand Up @@ -823,6 +859,17 @@ void aws_s3_init_default_signing_config(
const struct aws_byte_cursor region,
struct aws_credentials_provider *credentials_provider);

/**
* Return operation name for aws_s3_request_type,
* or empty string if the type doesn't map to an actual operation.
* For example:
* AWS_S3_REQUEST_TYPE_HEAD_OBJECT -> "HeadObject"
* AWS_S3_REQUEST_TYPE_UNKNOWN -> ""
* AWS_S3_REQUEST_TYPE_MAX -> ""
*/
AWS_S3_API
const char *aws_s3_request_type_operation_name(enum aws_s3_request_type type);

/**
* Add a reference, keeping this object alive.
* The reference must be released when you are done with it, or it's memory will never be cleaned up.
Expand Down Expand Up @@ -974,7 +1021,19 @@ int aws_s3_request_metrics_get_thread_id(const struct aws_s3_request_metrics *me
AWS_S3_API
int aws_s3_request_metrics_get_request_stream_id(const struct aws_s3_request_metrics *metrics, uint32_t *out_stream_id);

/* Get the request type from request metrics. */
/**
* Get the S3 operation name of the request (e.g. "HeadObject").
* If unavailable, AWS_ERROR_S3_METRIC_DATA_NOT_AVAILABLE will be raised.
* If available, out_operation_name will be set to a string.
* Be warned this string's lifetime is tied to the metrics object.
*/
AWS_S3_API
int aws_s3_request_metrics_get_operation_name(
const struct aws_s3_request_metrics *metrics,
const struct aws_string **out_operation_name);

/* Get the request type from request metrics.
* If you just need a string, aws_s3_request_metrics_get_operation_name() is more reliable. */
AWS_S3_API
void aws_s3_request_metrics_get_request_type(
Copy link
Member

Choose a reason for hiding this comment

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

IIRC we added this function and all the aws_s3_request_type stuff specifically for Mountpoint for use in metrics. If Mountpoint is still the only customer for it and you feel like you want to break it, it would be really easy for us to switch to this new interface instead, since we really only needed it to get operation names anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll keep the enums. I'm finding uses for both enum and string

const struct aws_s3_request_metrics *metrics,
Expand Down
27 changes: 8 additions & 19 deletions source/s3_auto_ranged_get.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ static void s_s3_auto_ranged_get_request_finished(
struct aws_s3_request *request,
int error_code);

static int s_s3_auto_ranged_get_request_type(const struct aws_s3_request *request);

static struct aws_s3_meta_request_vtable s_s3_auto_ranged_get_vtable = {
.update = s_s3_auto_ranged_get_update,
.send_request_finish = aws_s3_meta_request_send_request_finish_default,
Expand All @@ -39,21 +37,8 @@ static struct aws_s3_meta_request_vtable s_s3_auto_ranged_get_vtable = {
.finished_request = s_s3_auto_ranged_get_request_finished,
.destroy = s_s3_meta_request_auto_ranged_get_destroy,
.finish = aws_s3_meta_request_finish_default,
.get_request_type = s_s3_auto_ranged_get_request_type,
};

static int s_s3_auto_ranged_get_request_type(const struct aws_s3_request *request) {
switch (request->request_tag) {
case AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_HEAD_OBJECT:
return AWS_S3_REQUEST_TYPE_HEAD_OBJECT;
case AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_PART:
case AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_INITIAL_MESSAGE:
return AWS_S3_REQUEST_TYPE_GET_OBJECT;
}
AWS_ASSERT(false);
return AWS_S3_REQUEST_TYPE_MAX;
}

static int s_s3_auto_ranged_get_success_status(struct aws_s3_meta_request *meta_request) {
AWS_PRECONDITION(meta_request);

Expand Down Expand Up @@ -176,7 +161,8 @@ static bool s_s3_auto_ranged_get_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_HEAD_OBJECT,
0,
AWS_S3_REQUEST_TYPE_HEAD_OBJECT,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);

request->discovers_object_size = true;
Expand All @@ -197,7 +183,8 @@ static bool s_s3_auto_ranged_get_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_PART,
1,
AWS_S3_REQUEST_TYPE_GET_OBJECT,
1 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_PART_SIZE_RESPONSE_BODY);

request->ticket = ticket;
Expand All @@ -223,7 +210,8 @@ static bool s_s3_auto_ranged_get_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_INITIAL_MESSAGE,
0,
AWS_S3_REQUEST_TYPE_GET_OBJECT,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);

auto_ranged_get->synced_data.get_without_range_sent = true;
Expand Down Expand Up @@ -272,7 +260,8 @@ static bool s_s3_auto_ranged_get_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGE_GET_REQUEST_TYPE_PART,
auto_ranged_get->synced_data.num_parts_requested + 1,
AWS_S3_REQUEST_TYPE_GET_OBJECT,
auto_ranged_get->synced_data.num_parts_requested + 1 /*part_number*/,
AWS_S3_REQUEST_FLAG_PART_SIZE_RESPONSE_BODY);

request->ticket = ticket;
Expand Down
36 changes: 12 additions & 24 deletions source/s3_auto_ranged_put.c
Original file line number Diff line number Diff line change
Expand Up @@ -294,23 +294,6 @@ static int s_try_init_resume_state_from_persisted_data(
return AWS_OP_SUCCESS;
}

static int s_s3_auto_ranged_put_request_type(const struct aws_s3_request *request) {
switch (request->request_tag) {
case AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_LIST_PARTS:
return AWS_S3_REQUEST_TYPE_LIST_PARTS;
case AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_CREATE_MULTIPART_UPLOAD:
return AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD;
case AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART:
return AWS_S3_REQUEST_TYPE_UPLOAD_PART;
case AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_ABORT_MULTIPART_UPLOAD:
return AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD;
case AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD:
return AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD;
}
AWS_ASSERT(false);
return AWS_S3_REQUEST_TYPE_MAX;
}

static struct aws_s3_meta_request_vtable s_s3_auto_ranged_put_vtable = {
.update = s_s3_auto_ranged_put_update,
.send_request_finish = s_s3_auto_ranged_put_send_request_finish,
Expand All @@ -321,7 +304,6 @@ static struct aws_s3_meta_request_vtable s_s3_auto_ranged_put_vtable = {
.destroy = s_s3_meta_request_auto_ranged_put_destroy,
.finish = aws_s3_meta_request_finish_default,
.pause = s_s3_auto_ranged_put_pause,
.get_request_type = s_s3_auto_ranged_put_request_type,
};

/* Allocate a new auto-ranged put meta request */
Expand Down Expand Up @@ -480,7 +462,8 @@ static bool s_s3_auto_ranged_put_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_LIST_PARTS,
graebm marked this conversation as resolved.
Show resolved Hide resolved
0,
AWS_S3_REQUEST_TYPE_LIST_PARTS,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);

auto_ranged_put->synced_data.list_parts_state.started = true;
Expand All @@ -494,7 +477,8 @@ static bool s_s3_auto_ranged_put_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_LIST_PARTS,
0,
AWS_S3_REQUEST_TYPE_LIST_PARTS,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);
auto_ranged_put->synced_data.list_parts_state.continues = false;
goto has_work_remaining;
Expand All @@ -510,7 +494,8 @@ static bool s_s3_auto_ranged_put_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_CREATE_MULTIPART_UPLOAD,
0,
AWS_S3_REQUEST_TYPE_CREATE_MULTIPART_UPLOAD,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);

auto_ranged_put->synced_data.create_multipart_upload_sent = true;
Expand Down Expand Up @@ -564,7 +549,8 @@ static bool s_s3_auto_ranged_put_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_PART,
0,
AWS_S3_REQUEST_TYPE_UPLOAD_PART,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_PART_SIZE_REQUEST_BODY);

request->part_number = auto_ranged_put->threaded_update_data.next_part_number;
Expand Down Expand Up @@ -610,7 +596,8 @@ static bool s_s3_auto_ranged_put_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_COMPLETE_MULTIPART_UPLOAD,
0,
AWS_S3_REQUEST_TYPE_COMPLETE_MULTIPART_UPLOAD,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS);

auto_ranged_put->synced_data.complete_multipart_upload_sent = true;
Expand Down Expand Up @@ -675,7 +662,8 @@ static bool s_s3_auto_ranged_put_update(
request = aws_s3_request_new(
meta_request,
AWS_S3_AUTO_RANGED_PUT_REQUEST_TAG_ABORT_MULTIPART_UPLOAD,
0,
AWS_S3_REQUEST_TYPE_ABORT_MULTIPART_UPLOAD,
0 /*part_number*/,
AWS_S3_REQUEST_FLAG_RECORD_RESPONSE_HEADERS | AWS_S3_REQUEST_FLAG_ALWAYS_SEND);

auto_ranged_put->synced_data.abort_multipart_upload_sent = true;
Expand Down
Loading
Loading