-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: surface HTTP connection manager metrics #88
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"id": "11a891eb-b51f-4508-9010-f17d35ce96a3", | ||
"type": "feature", | ||
"description": "Surface HTTP connection manager metrics", | ||
"issues": [ | ||
"awslabs/smithy-kotlin#893" | ||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
/* | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
package aws.sdk.kotlin.crt.http | ||
|
||
public data class HttpManagerMetrics( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: Are these connection manager metrics? If so 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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). |
||
public val availableConcurrency: Long, | ||
public val pendingConcurrencyAcquires: Long, | ||
public val leasedConcurrency: Long, | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Wouldn't
metrics
suffice here given it's namespaced underHttpClientConnectionManager
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).