-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Opentelemetry tracing #2577
Opentelemetry tracing #2577
Conversation
generated/src/aws-cpp-sdk-AWSMigrationHub/source/MigrationHubClient.cpp
Outdated
Show resolved
Hide resolved
return AssociateCreatedArtifactOutcome(MakeRequest(request, endpointResolutionOutcome.GetResult(), Aws::Http::HttpMethod::HTTP_POST, Aws::Auth::SIGV4_SIGNER)); | ||
auto tracer = m_telemetryProvider->getTracer(this->GetServiceClientName(), {}); | ||
auto span = tracer->CreateSpan(Aws::String(this->GetServiceClientName()) + ".AssociateCreatedArtifact", | ||
{{ "rpc.method", request.GetServiceRequestName() }, { "rpc.service", this->GetServiceClientName() }, { "rpc.system", "aws-api" }}, |
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.
if this->GetServiceClientName()
is used to get over-writable service name (that can be updated by customer to update user agent) - then good
otherwise, SERVICE_NAME static variable will serve this purpose.
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.
I dont think we will be able to use SERVICE_NAME because we add the service name dimensions in XmlClient/JsonClient/AwsClient where it is not accessible. Also SERVICE_NAME and GetServiceClientName could and do return different values, and they are needed to be the same for dimensions to work correctly.
that function is defined as
inline virtual const char* GetServiceClientName() const { return m_serviceName.c_str(); }
so i think it is ok to leave as is, let me know what you think though
src/aws-cpp-sdk-core/source/smithy/tracing/impl/opentelemetry/MeterAdapters.cpp
Outdated
Show resolved
Hide resolved
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.
Looks good.
Please also add doc comments for classes and methods definitions in headers.
e5224e5
to
0521c7b
Compare
…jects tracing and metrics into the SDK
0521c7b
to
c5983b8
Compare
Description of changes:
Creates a Smithy Reference Architecture interface for emitting traces and metrics. This will allow for client users to more easily monitor the telemetry of the SDK inside a running application to find metrics. We also provide a Open telemetry implementation of metrics and tracing so that a user could see immediate value. For example a user could configure the SDK as such
and receive the following output
While the above uses the OStream exporter, the SDK can be configured with any of the open telemetry exporters. In specific this can be used with the AWS Distro for OpenTelemetry to report metrics and traces straight to AWS.
Check all that applies:
Check which platforms you have built SDK on to verify the correctness of this PR.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.