refactor: instrument using new telemetry APIs #866
Conversation
| writer.write("\n\n") | ||
| writer.write("public const val ServiceId: String = #S", ctx.settings.sdkId) | ||
| writer.write("public const val SdkVersion: String = #S", ctx.settings.pkg.version) | ||
| writer.write("\n\n") | ||
|
|
There was a problem hiding this comment.
Question: Why did these move? Should ServiceApiVersion move as well?
There was a problem hiding this comment.
I don't remember now but there was a reason 🤔 . ServiceApiVersion is AWS specific, the other two are not.
| write("val span = tracer.createSpan(") | ||
| .indent() | ||
| .write("#S,", "\${ServiceId}.\${operation}") | ||
| .withBlock("#T{", "},", RuntimeTypes.Core.Utils.attributesOf) { | ||
| write("#S to ServiceId", "rpc.service") | ||
| write("#S to operation", "rpc.method") | ||
| writer.declareSection(OperationSpanAttributes) | ||
| } | ||
| } | ||
| .write("#T.CLIENT,", RuntimeTypes.Observability.TelemetryApi.SpanKind) | ||
| .write("parentCtx") | ||
| .dedent() | ||
| .write(")") |
There was a problem hiding this comment.
Nit: Could use withBlock.
| } | ||
| } | ||
| .write("#T.CLIENT,", RuntimeTypes.Observability.TelemetryApi.SpanKind) | ||
| .write("parentCtx") |
| public interface Builder { | ||
| /** | ||
| * The [TelemetryProvider] to instrument the component with. | ||
| */ | ||
| public var telemetryProvider: TelemetryProvider? | ||
| } |
There was a problem hiding this comment.
Question: We intend to provide a default value of TelemetryProvider.None right? Why leave the var nullable?
There was a problem hiding this comment.
It's on the builder, this is how most of our config builders work right? TelemetryConfig.telemetryProvider is non nullable and will get the default.
| .call { renderAdditionalMethods(writer) } | ||
| .write("") |
There was a problem hiding this comment.
Is this new .write("") call necessary, it wasn't there before?
There was a problem hiding this comment.
well technically none of them are necessary since it's formatting, can remove though
|
SonarCloud Quality Gate failed.
|
Issue #
N/A
Description of changes
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.