addressed new review comments

Author: Krish

include/aws/s3/private/s3_request.h

@@ -144,7 +144,7 @@ struct aws_s3_request_metrics {
         /* The host address of the request. */
         struct aws_string *host_address;
         /* The the request ID header value. */
-        struct aws_string *request_attempt_id;
+        struct aws_string *request_id;
         /* The the extended request ID header value. */
         struct aws_string *amz_id_2;
         /* S3 operation name for the request */
@@ -157,9 +157,9 @@ struct aws_s3_request_metrics {
         /* The IP address of the request connected to */
         struct aws_string *ip_address;
         /* The pointer to the connection that request was made from */
-        void *connection_id;
+        void *connection_ptr;
         /* The pointer to the request that the request attempt was made from */
-        void *request_id;
+        void *request_ptr;
         /* The aws_thread_id_t to the thread that request ran on */
         aws_thread_id_t thread_id;
         /* The stream-id, which is the idex when the stream was activated. */

include/aws/s3/s3_client.h

@@ -1386,9 +1386,9 @@ struct aws_s3_request_metrics *aws_s3_request_metrics_release(struct aws_s3_requ
  * object.
  **/
 AWS_S3_API
-int aws_s3_request_metrics_get_request_attempt_id(
+int aws_s3_request_metrics_get_request_id(
     const struct aws_s3_request_metrics *metrics,
-    const struct aws_string **out_request_attempt_id);
+    const struct aws_string **out_request_id);
 
 /**
  * Get the extended request ID from aws_s3_request_metrics.
@@ -1577,11 +1577,11 @@ int aws_s3_request_metrics_get_ip_address(
 /* Get the id of connection that request was made from. AWS_ERROR_S3_METRIC_DATA_NOT_AVAILABLE will be raised if data
  * not available */
 AWS_S3_API
-int aws_s3_request_metrics_get_connection_id(const struct aws_s3_request_metrics *metrics, size_t *out_connection_id);
+int aws_s3_request_metrics_get_connection_ptr(const struct aws_s3_request_metrics *metrics, size_t *out_connection_ptr);
 
 /* Get the pointer to the request that attempt was made from. Always available. */
 AWS_S3_API
-int aws_s3_request_metrics_get_request_id(const struct aws_s3_request_metrics *metrics, size_t *out_request_id);
+int aws_s3_request_metrics_get_request_ptr(const struct aws_s3_request_metrics *metrics, size_t *out_request_ptr);
 
 /* Get the thread ID of the thread that request was made from. AWS_ERROR_S3_METRIC_DATA_NOT_AVAILABLE will be raised if
  * data not available */

source/s3_meta_request.c

@@ -1374,7 +1374,7 @@ static int s_s3_meta_request_incoming_headers(
         if (request->send_data.request_id == NULL && aws_byte_cursor_eq(name, &g_request_id_header_name)) {
             request->send_data.request_id = aws_string_new_from_cursor(connection->request->allocator, value);
             if (collect_metrics) {
-                request->send_data.metrics->req_resp_info_metrics.request_attempt_id =
+                request->send_data.metrics->req_resp_info_metrics.request_id =
                     aws_string_new_from_cursor(connection->request->allocator, value);
             }
         }
@@ -1534,7 +1534,7 @@ static void s_s3_meta_request_stream_metrics(
     s3_metrics->crt_info_metrics.stream_id = http_metrics->stream_id;
 
     /* Also related metrics from the request/response. */
-    s3_metrics->crt_info_metrics.connection_id = (void *)connection->http_connection;
+    s3_metrics->crt_info_metrics.connection_ptr = (void *)connection->http_connection;
     const struct aws_socket_endpoint *endpoint = aws_http_connection_get_remote_endpoint(connection->http_connection);
     request->send_data.metrics->crt_info_metrics.ip_address =
         aws_string_new_from_c_str(request->allocator, endpoint->address);

source/s3_request.c

@@ -144,7 +144,7 @@ void aws_s3_request_setup_send_data(struct aws_s3_request *request, struct aws_h
     }
 
     /* set pointer to request */
-    request->send_data.metrics->crt_info_metrics.request_id = request;
+    request->send_data.metrics->crt_info_metrics.request_ptr = request;
 
     aws_http_message_acquire(message);
 }
@@ -223,7 +223,7 @@ static void s_s3_request_metrics_destroy(void *arg) {
     aws_http_headers_release(metrics->req_resp_info_metrics.response_headers);
     aws_string_destroy(metrics->req_resp_info_metrics.request_path_query);
     aws_string_destroy(metrics->req_resp_info_metrics.host_address);
-    aws_string_destroy(metrics->req_resp_info_metrics.request_attempt_id);
+    aws_string_destroy(metrics->req_resp_info_metrics.request_id);
     aws_string_destroy(metrics->req_resp_info_metrics.amz_id_2);
     aws_string_destroy(metrics->req_resp_info_metrics.operation_name);
     aws_string_destroy(metrics->crt_info_metrics.ip_address);
@@ -296,15 +296,15 @@ struct aws_s3_request_metrics *aws_s3_request_metrics_release(struct aws_s3_requ
     return NULL;
 }
 
-int aws_s3_request_metrics_get_request_attempt_id(
+int aws_s3_request_metrics_get_request_id(
     const struct aws_s3_request_metrics *metrics,
-    const struct aws_string **out_request_attempt_id) {
+    const struct aws_string **out_request_id) {
     AWS_PRECONDITION(metrics);
-    AWS_PRECONDITION(out_request_attempt_id);
-    if (metrics->req_resp_info_metrics.request_attempt_id == NULL) {
+    AWS_PRECONDITION(out_request_id);
+    if (metrics->req_resp_info_metrics.request_id == NULL) {
         return aws_raise_error(AWS_ERROR_S3_METRIC_DATA_NOT_AVAILABLE);
     }
-    *out_request_attempt_id = metrics->req_resp_info_metrics.request_attempt_id;
+    *out_request_id = metrics->req_resp_info_metrics.request_id;
     return AWS_OP_SUCCESS;
 }
 
@@ -572,20 +572,20 @@ int aws_s3_request_metrics_get_ip_address(
     return AWS_OP_SUCCESS;
 }
 
-int aws_s3_request_metrics_get_connection_id(const struct aws_s3_request_metrics *metrics, size_t *connection_id) {
+int aws_s3_request_metrics_get_connection_ptr(const struct aws_s3_request_metrics *metrics, size_t *connection_ptr) {
     AWS_PRECONDITION(metrics);
-    AWS_PRECONDITION(connection_id);
-    if (metrics->crt_info_metrics.connection_id == NULL) {
+    AWS_PRECONDITION(connection_ptr);
+    if (metrics->crt_info_metrics.connection_ptr == NULL) {
         return aws_raise_error(AWS_ERROR_S3_METRIC_DATA_NOT_AVAILABLE);
     }
-    *connection_id = (size_t)metrics->crt_info_metrics.connection_id;
+    *connection_ptr = (size_t)metrics->crt_info_metrics.connection_ptr;
     return AWS_OP_SUCCESS;
 }
 
-int aws_s3_request_metrics_get_request_id(const struct aws_s3_request_metrics *metrics, size_t *out_request_id) {
+int aws_s3_request_metrics_get_request_ptr(const struct aws_s3_request_metrics *metrics, size_t *out_request_ptr) {
     AWS_PRECONDITION(metrics);
-    AWS_PRECONDITION(out_request_id);
-    *out_request_id = (size_t)metrics->crt_info_metrics.request_id;
+    AWS_PRECONDITION(out_request_ptr);
+    *out_request_ptr = (size_t)metrics->crt_info_metrics.request_ptr;
     return AWS_OP_SUCCESS;
 }
 

tests/s3_mock_server_tests.c

@@ -118,9 +118,9 @@ static int s_validate_time_metrics(struct aws_s3_request_metrics *metrics, bool
 static int s_validate_create_multipart_upload_metrics(struct aws_s3_request_metrics *metrics) {
     struct aws_http_headers *response_headers = NULL;
     ASSERT_SUCCESS(aws_s3_request_metrics_get_response_headers(metrics, &response_headers));
-    const struct aws_string *request_attempt_id = NULL;
-    ASSERT_SUCCESS(aws_s3_request_metrics_get_request_attempt_id(metrics, &request_attempt_id));
-    ASSERT_TRUE(aws_string_eq_c_str(request_attempt_id, "12345"));
+    const struct aws_string *request_id = NULL;
+    ASSERT_SUCCESS(aws_s3_request_metrics_get_request_id(metrics, &request_id));
+    ASSERT_TRUE(aws_string_eq_c_str(request_id, "12345"));
     const struct aws_string *ip_address = NULL;
     ASSERT_SUCCESS(aws_s3_request_metrics_get_ip_address(metrics, &ip_address));
     /* Should be default local ip for ipv6/ipv4 */
@@ -139,8 +139,8 @@ static int s_validate_create_multipart_upload_metrics(struct aws_s3_request_metr
     ASSERT_TRUE(host_address->len > 0);
     aws_thread_id_t thread_id = 0;
     ASSERT_SUCCESS(aws_s3_request_metrics_get_thread_id(metrics, &thread_id));
-    size_t connection_id = 0;
-    ASSERT_SUCCESS(aws_s3_request_metrics_get_connection_id(metrics, &connection_id));
+    size_t connection_ptr = 0;
+    ASSERT_SUCCESS(aws_s3_request_metrics_get_connection_ptr(metrics, &connection_ptr));
     ASSERT_UINT_EQUALS(AWS_ERROR_SUCCESS, aws_s3_request_metrics_get_error_code(metrics));
 
     /* Get all those time stamp */
@@ -291,6 +291,8 @@ static int s_validate_fail_metrics(struct aws_array_list *metrics_list, uint32_t
     /* First part fails 5 times */
     aws_array_list_get_at(metrics_list, &metrics, 1);
     ASSERT_TRUE(metrics->crt_info_metrics.error_code != AWS_ERROR_SUCCESS);
+    size_t request_ptr = (size_t)metrics->crt_info_metrics.request_ptr;
+    size_t request_start_time = metrics->time_metrics.s3_request_first_attempt_start_timestamp_ns;
     for (size_t i = 1; i < 6; i++) {
         aws_array_list_get_at(metrics_list, &metrics2, i + 1);
         ASSERT_INT_EQUALS(
@@ -299,6 +301,8 @@ static int s_validate_fail_metrics(struct aws_array_list *metrics_list, uint32_t
         ASSERT_INT_EQUALS(metrics->crt_info_metrics.retry_attempt + 1, metrics2->crt_info_metrics.retry_attempt);
         ASSERT_TRUE(metrics2->crt_info_metrics.error_code != AWS_ERROR_SUCCESS);
         ASSERT_SUCCESS(s_validate_upload_part_metrics(metrics, false));
+        ASSERT_INT_EQUALS(request_start_time, metrics2->time_metrics.s3_request_first_attempt_start_timestamp_ns);
+        ASSERT_TRUE(request_ptr == (size_t)metrics2->crt_info_metrics.request_ptr);
         metrics = metrics2;
     }
     ASSERT_SUCCESS(s_validate_upload_part_metrics(metrics, true));