Unverified Commit 00254621 authored by Aaron Todd's avatar Aaron Todd Committed by GitHub
Browse files

remove redundant span attributes and improve log output format (#4075) 

## Description
https://github.com/smithy-lang/smithy-rs/pull/4052 aligned our spans
closer to SRA but the output format resulted in `rpc.service` and
`rpc.method` on both the new generated operation span and the internal
orchestrator `invoke` span. This resulted in pretty long and clutter log
output. This PR cleans up the log output a bit to have only the top
level operation span include these attributes. This also fixes a subtle
bug where presigned requests wouldn't contain the same top level
operation span instrumentation because presigned requests make use of
`<Operation>::orchestrate_with_stop_point` and not
`<Operation>::orchestrate`. For IMDS and other "manually constructed"
operations, I've moved the `invoke` span instrumentation into
`Operation::invoke` to ensure we still capture that information in the
logs for those use cases.

Example output format:
```
2025-03-28T14:44:02.471068Z DEBUG S3.ListBuckets{rpc.service="S3" rpc.method="ListBuckets" sdk_invocation_id=4073472 rpc.system="aws-api"}:try_op: aws_smithy_runtime::cli
ent::orchestrator: beginning attempt #1
```

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] For changes to the smithy-rs codegen or runtime crates, I have
created a changelog entry Markdown file in the `.changelog` directory,
specifying "client," "server," or both in the `applies_to` key.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent f50c8eb9

.changelog/1743173123.md

0 → 100644
+11 −0
Original line number Diff line number Diff line 
---

applies_to:

- client

authors:

- aajtodd

references: []

breaking: false

new_feature: false

bug_fix: false

---

remove redundant span attributes and improve log output format

aws/rust-runtime/Cargo.lock

+2 −2
Original line number Diff line number Diff line
@@ -282,7 +282,7 @@ dependencies = [ 


[[package]]

name = "aws-smithy-http-client"

version = "1.0.0"

version = "1.0.1"

dependencies = [

 "aws-smithy-async",

 "aws-smithy-runtime-api",
@@ -325,7 +325,7 @@ dependencies = [ 


[[package]]

name = "aws-smithy-runtime"

version = "1.8.1"

version = "1.8.2"

dependencies = [

 "aws-smithy-async",

 "aws-smithy-http",

aws/sdk/integration-tests/telemetry/tests/spans.rs

+0 −19
Original line number Diff line number Diff line
@@ -62,7 +62,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
    let _guard = tracing::subscriber::set_default(subscriber);



    const OPERATION_NAME: &str = "S3.GetObject";

    const INVOKE: &str = "invoke";

    const TRY_OP: &str = "try_op";

    const TRY_ATTEMPT: &str = "try_attempt";
@@ -70,7 +69,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("apply_configuration")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .was_closed_exactly(1)

        .finalize();
@@ -78,7 +76,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("serialization")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .with_parent_name(TRY_OP)

        .was_closed_exactly(1)

        .finalize();
@@ -87,7 +84,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("orchestrate_endpoint")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .with_parent_name(TRY_OP)

        .with_parent_name(TRY_ATTEMPT)

        .was_closed_exactly(1)
@@ -97,7 +93,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("lazy_load_identity")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .with_parent_name(TRY_OP)

        .with_parent_name(TRY_ATTEMPT)

        .was_closed_exactly(1)
@@ -107,7 +102,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("deserialize_streaming")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .with_parent_name(TRY_OP)

        .with_parent_name(TRY_ATTEMPT)

        .was_closed_exactly(1)
@@ -117,7 +111,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("deserialization")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .with_parent_name(TRY_OP)

        .with_parent_name(TRY_ATTEMPT)

        .was_closed_exactly(1)
@@ -128,7 +121,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .with_name(TRY_ATTEMPT)

        .with_span_field("attempt")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .with_parent_name(TRY_OP)

        .was_closed_exactly(1)

        .finalize();
@@ -137,7 +129,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("finally_attempt")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .with_parent_name(TRY_OP)

        .was_closed_exactly(1)

        .finalize();
@@ -146,7 +137,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name(TRY_OP)

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .was_closed_exactly(1)

        .finalize();
@@ -154,14 +144,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
        .build()

        .with_name("finally_op")

        .with_parent_name(OPERATION_NAME)

        .with_parent_name(INVOKE)

        .was_closed_exactly(1)

        .finalize();



    let invoke = assertion_registry

        .build()

        .with_name(INVOKE)

        .with_parent_name(OPERATION_NAME)

        .was_closed_exactly(1)

        .finalize();
@@ -184,7 +166,6 @@ async fn all_expected_operation_spans_emitted_with_correct_nesting() { 
    finally_attempt.assert();

    try_op.assert();

    finally_op.assert();

    invoke.assert();

    operation.assert();

}

codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/OperationGenerator.kt

+12 −11
Original line number Diff line number Diff line
@@ -138,17 +138,7 @@ open class OperationGenerator( 
                            err.downcast::<#{OperationError}>().expect("correct error type")

                        })

                    };

                    use #{Tracing}::Instrument;

                    let context = Self::orchestrate_with_stop_point(runtime_plugins, input, #{StopPoint}::None)

                        // Create a parent span for the entire operation. Includes a random, internal-only,

                        // seven-digit ID for the operation orchestration so that it can be correlated in the logs.

                        .instrument(#{Tracing}::debug_span!(

                                "$serviceName.$operationName",

                                "rpc.service" = ${serviceName.dq()},

                                "rpc.method" = ${operationName.dq()},

                                "sdk_invocation_id" = #{FastRand}::u32(1_000_000..10_000_000),

                                #{AdditionalSpanFields}

                            ))

                        .await

                        .map_err(map_err)?;

                    let output = context.finalize().map_err(map_err)?;
@@ -161,13 +151,24 @@ open class OperationGenerator( 
                    stop_point: #{StopPoint},

                ) -> #{Result}<#{InterceptorContext}, #{SdkError}<#{Error}, #{HttpResponse}>> {

                    let input = #{Input}::erase(input);

                    use #{Tracing}::Instrument;

                    #{invoke_with_stop_point}(

                        ${serviceName.dq()},

                        ${operationName.dq()},

                        input,

                        runtime_plugins,

                        stop_point

                    ).await

                    )

                    // Create a parent span for the entire operation. Includes a random, internal-only,

                    // seven-digit ID for the operation orchestration so that it can be correlated in the logs.

                    .instrument(#{Tracing}::debug_span!(

                            "$serviceName.$operationName",

                            "rpc.service" = ${serviceName.dq()},

                            "rpc.method" = ${operationName.dq()},

                            "sdk_invocation_id" = #{FastRand}::u32(1_000_000..10_000_000),

                            #{AdditionalSpanFields}

                        ))

                    .await

                }



                pub(crate) fn operation_runtime_plugins(

rust-runtime/Cargo.lock

+3 −3
Original line number Diff line number Diff line
@@ -463,7 +463,7 @@ dependencies = [ 


[[package]]

name = "aws-smithy-http-server"

version = "0.64.0"

version = "0.65.0"

dependencies = [

 "aws-smithy-cbor",

 "aws-smithy-http",
@@ -494,7 +494,7 @@ dependencies = [ 


[[package]]

name = "aws-smithy-http-server-python"

version = "0.64.0"

version = "0.65.0"

dependencies = [

 "aws-smithy-http",

 "aws-smithy-http-server",
@@ -604,7 +604,7 @@ dependencies = [ 


[[package]]

name = "aws-smithy-runtime"

version = "1.8.1"

version = "1.8.2"

dependencies = [

 "approx",

 "aws-smithy-async",