feat: surface HTTP connection manager metrics#88
Conversation
|
aajtodd
left a comment
aajtodd
left a comment
There was a problem hiding this comment.
Couple questions/nit but looks good.
| /** | ||
| * The active metrics for this connection manager | ||
| */ | ||
| public val managerMetrics: HttpManagerMetrics |
There was a problem hiding this comment.
nit: Wouldn't metrics suffice here given it's namespaced under HttpClientConnectionManager
There was a problem hiding this comment.
Similar to other Java→Kotlin mapping classes, I chose to keep this the same as the underlying property. If you feel strongly I can change it to be simpler (which I would generally prefer when not mapping existing classes).
| */ | ||
| package aws.sdk.kotlin.crt.http | ||
|
|
||
| public data class HttpManagerMetrics( |
There was a problem hiding this comment.
question: Are these connection manager metrics? If so HttpManager seems ambiguous and should probably be HttpConnectionManagerMetrics or HttpConnectionMetrics
fix: Missing docs on these metrics. I know they are meant to match SRA but probably should document them still and any differences/quirks that may lie herein.
There was a problem hiding this comment.
Similar to other Java→Kotlin mapping classes, I chose to keep this the same as the underlying class. If you feel strongly I can change it to be clearer (which I would generally prefer when not mapping existing classes).
3 participants
Issue #, if available:
smithy-lang/smithy-kotlin#893
Description of changes:
This change makes HTTP manager metrics available in the Kotlin API.
Companion PR: smithy-lang/smithy-kotlin#1017
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.