Skip to content

Commit 799987c

Browse files
DmitriyMusatkinDmitriyMusatkin
authored
Fix recording of early request metrics (#542)
1 parent 3afa5d0 commit 799987c

File tree

4 files changed

+92
-50
lines changed

4 files changed

+92
-50
lines changed

‎include/aws/s3/private/s3_request.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ struct aws_s3_request_metrics {
120120
uint32_t stream_id;
121121
/* CRT error code when the aws_s3_request finishes. */
122122
int error_code;
123+
/* Retry attempt. */
124+
uint32_t retry_attempt;
123125
} crt_info_metrics;
124126

125127
struct aws_ref_count ref_count;
@@ -306,10 +308,7 @@ AWS_S3_API
306308
struct aws_s3_request *aws_s3_request_release(struct aws_s3_request *request);
307309

308310
AWS_S3_API
309-
struct aws_s3_request_metrics *aws_s3_request_metrics_new(
310-
struct aws_allocator *allocator,
311-
const struct aws_s3_request *request,
312-
const struct aws_http_message *message);
311+
struct aws_s3_request_metrics *aws_s3_request_metrics_new(struct aws_allocator *allocator);
313312

314313
AWS_EXTERN_C_END
315314

‎include/aws/s3/s3_client.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1533,6 +1533,10 @@ void aws_s3_request_metrics_get_request_type(
15331533
AWS_S3_API
15341534
int aws_s3_request_metrics_get_error_code(const struct aws_s3_request_metrics *metrics);
15351535

1536+
/* Get retry attempt from request metrics. */
1537+
AWS_S3_API
1538+
uint32_t aws_s3_request_metrics_get_retry_attempt(const struct aws_s3_request_metrics *metrics);
1539+
15361540
AWS_EXTERN_C_END
15371541
AWS_POP_SANE_WARNING_LEVEL
15381542

‎source/s3_request.c‎

Lines changed: 68 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -40,44 +40,83 @@ struct aws_s3_request *aws_s3_request_new(
4040
request->should_allocate_buffer_from_pool = (flags & AWS_S3_REQUEST_FLAG_ALLOCATE_BUFFER_FROM_POOL) != 0;
4141
request->always_send = (flags & AWS_S3_REQUEST_FLAG_ALWAYS_SEND) != 0;
4242

43+
request->send_data.metrics = aws_s3_request_metrics_new(request->allocator);
44+
4345
return request;
4446
}
4547

48+
static void s_populate_metrics_from_message(struct aws_s3_request *request, struct aws_http_message *message) {
49+
struct aws_byte_cursor out_path;
50+
AWS_ZERO_STRUCT(out_path);
51+
int err = aws_http_message_get_request_path(message, &out_path);
52+
/* If there is no path of the message, it should be a program error. */
53+
AWS_ASSERT(!err);
54+
request->send_data.metrics->req_resp_info_metrics.request_path_query =
55+
aws_string_new_from_cursor(request->send_data.metrics->allocator, &out_path);
56+
AWS_ASSERT(request->send_data.metrics->req_resp_info_metrics.request_path_query != NULL);
57+
58+
/* Get the host header value */
59+
struct aws_byte_cursor host_header_value;
60+
AWS_ZERO_STRUCT(host_header_value);
61+
struct aws_http_headers *message_headers = aws_http_message_get_headers(message);
62+
AWS_ASSERT(message_headers);
63+
err = aws_http_headers_get(message_headers, g_host_header_name, &host_header_value);
64+
AWS_ASSERT(!err);
65+
request->send_data.metrics->req_resp_info_metrics.host_address =
66+
aws_string_new_from_cursor(request->send_data.metrics->allocator, &host_header_value);
67+
AWS_ASSERT(request->send_data.metrics->req_resp_info_metrics.host_address != NULL);
68+
69+
request->send_data.metrics->req_resp_info_metrics.request_type = request->request_type;
70+
request->send_data.metrics->req_resp_info_metrics.operation_name =
71+
aws_string_new_from_string(request->send_data.metrics->allocator, request->operation_name);
72+
73+
(void)err;
74+
}
75+
4676
void aws_s3_request_setup_send_data(struct aws_s3_request *request, struct aws_http_message *message) {
4777
AWS_PRECONDITION(request);
4878
AWS_PRECONDITION(message);
4979

50-
if (request != NULL && request->send_data.metrics != NULL) {
51-
/* If there is a metrics from previous attempt, complete it now. */
52-
struct aws_s3_request_metrics *metric = request->send_data.metrics;
53-
aws_high_res_clock_get_ticks((uint64_t *)&metric->time_metrics.end_timestamp_ns);
54-
metric->time_metrics.total_duration_ns =
55-
metric->time_metrics.end_timestamp_ns - metric->time_metrics.start_timestamp_ns;
56-
57-
struct aws_s3_meta_request *meta_request = request->meta_request;
58-
if (meta_request != NULL && meta_request->telemetry_callback != NULL) {
59-
60-
aws_s3_meta_request_lock_synced_data(meta_request);
61-
struct aws_s3_meta_request_event event = {.type = AWS_S3_META_REQUEST_EVENT_TELEMETRY};
62-
event.u.telemetry.metrics = aws_s3_request_metrics_acquire(metric);
63-
aws_s3_meta_request_add_event_for_delivery_synced(meta_request, &event);
64-
aws_s3_meta_request_unlock_synced_data(meta_request);
80+
/* If this is not the first time request is prepared, e.g. this is a retry,
81+
* send out previous metrics and reinitialize metrics structure.
82+
*/
83+
if (request->num_times_prepared > 0) {
84+
if (request != NULL && request->send_data.metrics != NULL) {
85+
/* If there is a metrics from previous attempt, complete it now. */
86+
struct aws_s3_request_metrics *metric = request->send_data.metrics;
87+
aws_high_res_clock_get_ticks((uint64_t *)&metric->time_metrics.end_timestamp_ns);
88+
metric->time_metrics.total_duration_ns =
89+
metric->time_metrics.end_timestamp_ns - metric->time_metrics.start_timestamp_ns;
90+
91+
struct aws_s3_meta_request *meta_request = request->meta_request;
92+
if (meta_request != NULL && meta_request->telemetry_callback != NULL) {
93+
94+
aws_s3_meta_request_lock_synced_data(meta_request);
95+
struct aws_s3_meta_request_event event = {.type = AWS_S3_META_REQUEST_EVENT_TELEMETRY};
96+
event.u.telemetry.metrics = aws_s3_request_metrics_acquire(metric);
97+
aws_s3_meta_request_add_event_for_delivery_synced(meta_request, &event);
98+
aws_s3_meta_request_unlock_synced_data(meta_request);
99+
}
100+
request->send_data.metrics = aws_s3_request_metrics_release(metric);
65101
}
66-
request->send_data.metrics = aws_s3_request_metrics_release(metric);
102+
aws_s3_request_clean_up_send_data(request);
103+
104+
request->send_data.metrics = aws_s3_request_metrics_new(request->allocator);
105+
request->send_data.metrics->crt_info_metrics.retry_attempt = request->num_times_prepared;
67106
}
68-
aws_s3_request_clean_up_send_data(request);
69107

70108
request->send_data.message = message;
71-
request->send_data.metrics = aws_s3_request_metrics_new(request->allocator, request, message);
109+
s_populate_metrics_from_message(request, message);
72110
/* Start the timestamp */
73111
aws_high_res_clock_get_ticks((uint64_t *)&request->send_data.metrics->time_metrics.start_timestamp_ns);
74-
75112
aws_http_message_acquire(message);
76113
}
77114

78115
static void s_s3_request_clean_up_send_data_message(struct aws_s3_request *request) {
79116
AWS_PRECONDITION(request);
80117

118+
request->send_data.metrics = aws_s3_request_metrics_release(request->send_data.metrics);
119+
81120
struct aws_http_message *message = request->send_data.message;
82121

83122
if (message == NULL) {
@@ -91,7 +130,8 @@ static void s_s3_request_clean_up_send_data_message(struct aws_s3_request *reque
91130
void aws_s3_request_clean_up_send_data(struct aws_s3_request *request) {
92131
AWS_PRECONDITION(request);
93132
/* The metrics should be collected and provided to user before reaching here */
94-
AWS_FATAL_ASSERT(request->send_data.metrics == NULL);
133+
AWS_FATAL_ASSERT(
134+
request->send_data.metrics == NULL || request->send_data.metrics->time_metrics.start_timestamp_ns == -1);
95135

96136
s_s3_request_clean_up_send_data_message(request);
97137

@@ -152,33 +192,10 @@ static void s_s3_request_metrics_destroy(void *arg) {
152192
aws_mem_release(metrics->allocator, metrics);
153193
}
154194

155-
struct aws_s3_request_metrics *aws_s3_request_metrics_new(
156-
struct aws_allocator *allocator,
157-
const struct aws_s3_request *request,
158-
const struct aws_http_message *message) {
195+
struct aws_s3_request_metrics *aws_s3_request_metrics_new(struct aws_allocator *allocator) {
159196

160197
struct aws_s3_request_metrics *metrics = aws_mem_calloc(allocator, 1, sizeof(struct aws_s3_request_metrics));
161198
metrics->allocator = allocator;
162-
struct aws_byte_cursor out_path;
163-
AWS_ZERO_STRUCT(out_path);
164-
int err = aws_http_message_get_request_path(message, &out_path);
165-
/* If there is no path of the message, it should be a program error. */
166-
AWS_ASSERT(!err);
167-
metrics->req_resp_info_metrics.request_path_query = aws_string_new_from_cursor(allocator, &out_path);
168-
AWS_ASSERT(metrics->req_resp_info_metrics.request_path_query != NULL);
169-
170-
/* Get the host header value */
171-
struct aws_byte_cursor host_header_value;
172-
AWS_ZERO_STRUCT(host_header_value);
173-
struct aws_http_headers *message_headers = aws_http_message_get_headers(message);
174-
AWS_ASSERT(message_headers);
175-
err = aws_http_headers_get(message_headers, g_host_header_name, &host_header_value);
176-
AWS_ASSERT(!err);
177-
metrics->req_resp_info_metrics.host_address = aws_string_new_from_cursor(allocator, &host_header_value);
178-
AWS_ASSERT(metrics->req_resp_info_metrics.host_address != NULL);
179-
180-
metrics->req_resp_info_metrics.request_type = request->request_type;
181-
metrics->req_resp_info_metrics.operation_name = aws_string_new_from_string(allocator, request->operation_name);
182199

183200
metrics->time_metrics.start_timestamp_ns = -1;
184201
metrics->time_metrics.end_timestamp_ns = -1;
@@ -201,11 +218,11 @@ struct aws_s3_request_metrics *aws_s3_request_metrics_new(
201218

202219
metrics->req_resp_info_metrics.response_status = -1;
203220

204-
(void)err;
205-
aws_ref_count_init(&metrics->ref_count, metrics, s_s3_request_metrics_destroy);
221+
aws_ref_count_init(&metrics->ref_count, metrics, s_s3_request_metrics_destroy); /**/
206222

207223
return metrics;
208224
}
225+
209226
struct aws_s3_request_metrics *aws_s3_request_metrics_acquire(struct aws_s3_request_metrics *metrics) {
210227
if (!metrics) {
211228
return NULL;
@@ -539,3 +556,8 @@ int aws_s3_request_metrics_get_error_code(const struct aws_s3_request_metrics *m
539556
AWS_PRECONDITION(metrics);
540557
return metrics->crt_info_metrics.error_code;
541558
}
559+
560+
uint32_t aws_s3_request_metrics_get_retry_attempt(const struct aws_s3_request_metrics *metrics) {
561+
AWS_PRECONDITION(metrics);
562+
return metrics->crt_info_metrics.retry_attempt;
563+
}

‎tests/s3_tester.c‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,23 @@ static void s_s3_test_meta_request_telemetry(
245245
AWS_FATAL_ASSERT(during_time == (end_time - start_time));
246246
}
247247

248+
enum aws_s3_request_type request_type;
249+
aws_s3_request_metrics_get_request_type(metrics, &request_type);
250+
uint32_t retry_attempt = aws_s3_request_metrics_get_retry_attempt(metrics);
251+
252+
if (aws_s3_request_metrics_get_error_code(metrics) == 200 && retry_attempt == 0 &&
253+
(request_type == AWS_S3_REQUEST_TYPE_GET_OBJECT || request_type == AWS_S3_REQUEST_TYPE_UPLOAD_PART)) {
254+
uint64_t start_time = 0;
255+
uint64_t end_time = 0;
256+
uint64_t duration_time = 0;
257+
int error = 0;
258+
error |= aws_s3_request_metrics_get_mem_acquire_start_timestamp_ns(metrics, &start_time);
259+
error |= aws_s3_request_metrics_get_mem_acquire_end_timestamp_ns(metrics, &end_time);
260+
error |= aws_s3_request_metrics_get_mem_acquire_duration_ns(metrics, &duration_time);
261+
AWS_FATAL_ASSERT(error == AWS_OP_SUCCESS);
262+
AWS_FATAL_ASSERT(duration_time == (end_time - start_time));
263+
}
264+
248265
aws_s3_tester_lock_synced_data(tester);
249266
aws_array_list_push_back(&meta_request_test_results->synced_data.metrics, &metrics);
250267
aws_s3_request_metrics_acquire(metrics);

0 commit comments

Comments
 (0)