-
Notifications
You must be signed in to change notification settings - Fork 28
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
refactor!: rename metrics and remove redundant metrics #973
Conversation
{ | ||
"id": "c063605b-4702-4d8a-a841-2e0b7aa78e78", | ||
"type": "misc", | ||
"description": "**BREAKING**: Remove `smithy.client.request.size`, `smithy.client.response.size`, `smithy.client.retires` metrics. Rename all `smithy.client.*` metrics to `smithy.client.call.*`." |
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: There's a type here in retries
*/ | ||
public interface SpanContext { | ||
public companion object { | ||
public val Invalid: SpanContext = InvalidSpanContext |
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.
When would an "invalid" SpanContext ever be used/desired? If it's meant to be used as a default when no context is set, I'd avoid the word "invalid"
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.
https://opentelemetry.io/docs/specs/otel/trace/api/#isvalid
Spans may not be valid when there is no configured telemetry provider.
override val traceId: String = "00000000000000000000000000000000" | ||
override val spanId: String = "0000000000000000" |
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.
How are these values determined?
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.
OTel spec
126b232
to
ca7ce63
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Issue #
n/a
Description of changes
smithy.client.request.size
,smithy.client.response.size
, andsmithy.client.retries
metricssmithy.client.*
metrics tosmithy.client.call.*
SpanContext
toTraceSpan
setContext
from fluent log builder in-favor of explicitly setting span context K/V pairsBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.