Skip to content

Conversation

@TingDaoK
Copy link
Contributor

@TingDaoK TingDaoK commented Mar 31, 2022

Provide API to fetch metric for managers.

To support https://github.com/aws/aws-sdk-java-v2/blob/master/http-client-spi/src/main/java/software/amazon/awssdk/http/HttpMetric.java#L65

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

*/
AWS_HTTP_API
void aws_http_connection_manager_fetch_metric(
struct aws_http_connection_manager *manager,
Copy link
Contributor

Choose a reason for hiding this comment

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

const

Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: if you change "metric" -> "metrics" update this function name too

*/
AWS_HTTP_API
void aws_http2_stream_manager_fetch_metric(
struct aws_http2_stream_manager *http2_stream_manager,
Copy link
Contributor

Choose a reason for hiding this comment

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

const

Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: is this non-const because you need to acquire the lock?
If so, I'd be OK with making this const, and then just casting away the const-ness when you need to futz with the locks.

In principal it's const

*/
AWS_HTTP_API
void aws_http_connection_manager_fetch_metric(
struct aws_http_connection_manager *manager,
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: if you change "metric" -> "metrics" update this function name too

*/
AWS_HTTP_API
void aws_http2_stream_manager_fetch_metric(
struct aws_http2_stream_manager *http2_stream_manager,
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: is this non-const because you need to acquire the lock?
If so, I'd be OK with making this const, and then just casting away the const-ness when you need to futz with the locks.

In principal it's const

Copy link
Contributor

@graebm graebm left a comment

Choose a reason for hiding this comment

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

fix & ship

struct aws_http_manager_metric *out_metric);
void aws_http_connection_manager_fetch_metrics(
const struct aws_http_connection_manager *manager,
struct aws_http_manager_metrics *out_metric);
Copy link
Contributor

Choose a reason for hiding this comment

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

trivial: out_metric -> out_metrics

some compilers issue warnings if the param name is different between the .h and .c file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants