Skip to content

Commit

Permalink
Add Instrumentation documentation (#1772)
Browse files Browse the repository at this point in the history 
* Add documentation covering instrumentation approaches for Smithy Rust.

* Tweak the logging in the Pokemon service to better exemplify instrumentation.
    * Remove `TraceLayer` which violates sensitivity contract.
    * Switch to [Pretty](https://docs.rs/tracing-subscriber/latest/tracing_subscriber/fmt/format/struct.Pretty.html) logs to better showcase the outputs.

* Update [Logging in the Presence of Sensitive Data](#1536) checklist.
* Rename `logging` module to `instrumentation` to improve coherence across struct names and documentation.
  • Loading branch information
@hlbarber
hlbarber authored Sep 28, 2022
1 parent 7b96ae4 commit dcfb855
Show file tree
Hide file tree
Showing 23 changed files with 191 additions and 85 deletions.
42 changes: 21 additions & 21 deletions ...are/amazon/smithy/rust/codegen/server/smithy/generators/ServerHttpSensitivityGenerator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ class StatusCodeSensitivity(private val sensitive: Boolean, runtimeConfig: Runti
/** Returns the type of the `MakeFmt`. */
fun type(): Writable = writable {
if (sensitive) {
rustTemplate("#{SmithyHttpServer}::logging::MakeSensitive", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::MakeSensitive", *codegenScope)
} else {
rustTemplate("#{SmithyHttpServer}::logging::MakeIdentity", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::MakeIdentity", *codegenScope)
}
}

Expand Down Expand Up @@ -86,17 +86,17 @@ class LabelSensitivity(internal val labelIndexes: List<Int>, internal val greedy

/** Returns the type of the `MakeFmt`. */
fun type(): Writable = if (hasRedactions()) writable {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::uri::MakeLabel<fn(usize) -> bool>", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::uri::MakeLabel<fn(usize) -> bool>", *codegenScope)
} else writable {
rustTemplate("#{SmithyHttpServer}::logging::MakeIdentity", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::MakeIdentity", *codegenScope)
}

/** Returns the value of the `GreedyLabel`. */
private fun greedyLabelStruct(): Writable = writable {
if (greedyLabel != null) {
rustTemplate(
"""
Some(#{SmithyHttpServer}::logging::sensitivity::uri::GreedyLabel::new(${greedyLabel.segmentIndex}, ${greedyLabel.endOffset}))""",
Some(#{SmithyHttpServer}::instrumentation::sensitivity::uri::GreedyLabel::new(${greedyLabel.segmentIndex}, ${greedyLabel.endOffset}))""",
*codegenScope,
)
} else {
Expand Down Expand Up @@ -148,9 +148,9 @@ sealed class HeaderSensitivity(
/** Returns the type of the `MakeDebug`. */
fun type(): Writable = writable {
if (hasRedactions()) {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::headers::MakeHeaders<fn(&#{Http}::header::HeaderName) -> #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker>", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::headers::MakeHeaders<fn(&#{Http}::header::HeaderName) -> #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker>", *codegenScope)
} else {
rustTemplate("#{SmithyHttpServer}::logging::MakeIdentity", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::MakeIdentity", *codegenScope)
}
}

Expand Down Expand Up @@ -194,7 +194,7 @@ sealed class HeaderSensitivity(
let name_match = #{NameMatch:W};
#{SuffixAndValue:W}
#{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { key_suffix, value }
#{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { key_suffix, value }
}
} as fn(&_) -> _
""",
Expand Down Expand Up @@ -240,9 +240,9 @@ sealed class QuerySensitivity(
/** Returns the type of the `MakeFmt`. */
fun type(): Writable = writable {
if (hasRedactions()) {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::uri::MakeQuery<fn(&str) -> #{SmithyHttpServer}::logging::sensitivity::uri::QueryMarker>", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::uri::MakeQuery<fn(&str) -> #{SmithyHttpServer}::instrumentation::sensitivity::uri::QueryMarker>", *codegenScope)
} else {
rustTemplate("#{SmithyHttpServer}::logging::MakeIdentity", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::MakeIdentity", *codegenScope)
}
}

Expand All @@ -267,7 +267,7 @@ sealed class QuerySensitivity(
|name: &str| {
let key = $allKeysSensitive;
let value = #{Value:W};
#{SmithyHttpServer}::logging::sensitivity::uri::QueryMarker { key, value }
#{SmithyHttpServer}::instrumentation::sensitivity::uri::QueryMarker { key, value }
}
} as fn(&_) -> _
""",
Expand Down Expand Up @@ -296,7 +296,7 @@ data class MakeFmt(
* parts of the request/response HTTP as sensitive.
*
* These closures are provided to `RequestFmt` and `ResponseFmt` constructors, which in turn are provided to
* `InstrumentedOperation` to configure logging. These structures can be found in `aws_smithy_http_server::logging`.
* `InstrumentedOperation` to configure logging. These structures can be found in `aws_smithy_http_server::instrumentation`.
*
* See [Logging in the Presence of Sensitive Data](https://github.com/awslabs/smithy-rs/blob/main/design/src/rfcs/rfc0018_logging_sensitive.md)
* for more details.
Expand Down Expand Up @@ -458,10 +458,10 @@ class ServerHttpSensitivityGenerator(

private fun defaultRequestFmt(): MakeFmt {
val type = writable {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::DefaultRequestFmt", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::DefaultRequestFmt", *codegenScope)
}
val value = writable {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::RequestFmt::new()", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::RequestFmt::new()", *codegenScope)
}
return MakeFmt(type, value)
}
Expand All @@ -483,9 +483,9 @@ class ServerHttpSensitivityGenerator(
val type = writable {
rustTemplate(
"""
#{SmithyHttpServer}::logging::sensitivity::RequestFmt<
#{SmithyHttpServer}::instrumentation::sensitivity::RequestFmt<
#{HeaderType:W},
#{SmithyHttpServer}::logging::sensitivity::uri::MakeUri<
#{SmithyHttpServer}::instrumentation::sensitivity::uri::MakeUri<
#{LabelType:W},
#{QueryType:W}
>
Expand All @@ -498,18 +498,18 @@ class ServerHttpSensitivityGenerator(
)
}

val value = writable { rustTemplate("#{SmithyHttpServer}::logging::sensitivity::RequestFmt::new()", *codegenScope) } + headerSensitivity.setter() + labelSensitivity.setter() + querySensitivity.setters()
val value = writable { rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::RequestFmt::new()", *codegenScope) } + headerSensitivity.setter() + labelSensitivity.setter() + querySensitivity.setters()

return MakeFmt(type, value)
}

private fun defaultResponseFmt(): MakeFmt {
val type = writable {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::DefaultResponseFmt", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::DefaultResponseFmt", *codegenScope)
}

val value = writable {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::ResponseFmt::new()", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::ResponseFmt::new()", *codegenScope)
}
return MakeFmt(type, value)
}
Expand All @@ -527,15 +527,15 @@ class ServerHttpSensitivityGenerator(

val type = writable {
rustTemplate(
"#{SmithyHttpServer}::logging::sensitivity::ResponseFmt<#{HeaderType:W}, #{StatusType:W}>",
"#{SmithyHttpServer}::instrumentation::sensitivity::ResponseFmt<#{HeaderType:W}, #{StatusType:W}>",
"HeaderType" to headerSensitivity.type(),
"StatusType" to statusCodeSensitivity.type(),
*codegenScope,
)
}

val value = writable {
rustTemplate("#{SmithyHttpServer}::logging::sensitivity::ResponseFmt::new()", *codegenScope)
rustTemplate("#{SmithyHttpServer}::instrumentation::sensitivity::ResponseFmt::new()", *codegenScope)
} + headerSensitivity.setter() + statusCodeSensitivity.setter()

return MakeFmt(type, value)
Expand Down
2 changes: 1 addition & 1 deletion .../software/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationGenerator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class ServerOperationGenerator(
type Error = #{Error:W};
}
impl #{SmithyHttpServer}::logging::sensitivity::Sensitivity for $operationName {
impl #{SmithyHttpServer}::instrumentation::sensitivity::Sensitivity for $operationName {
type RequestFmt = #{RequestType:W};
type ResponseFmt = #{ResponseType:W};
Expand Down
2 changes: 1 addition & 1 deletion ...e/amazon/smithy/rust/codegen/server/smithy/generators/ServerOperationRegistryGenerator.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ ${operationImplementationStubs(operations)}
let svc = #{ServerOperationHandler}::operation(registry.$operationName);
let request_fmt = #{RequestFmt:W};
let response_fmt = #{ResponseFmt:W};
let svc = #{SmithyHttpServer}::logging::InstrumentOperation::new(svc, "$operationName").request_fmt(request_fmt).response_fmt(response_fmt);
let svc = #{SmithyHttpServer}::instrumentation::InstrumentOperation::new(svc, "$operationName").request_fmt(request_fmt).response_fmt(response_fmt);
(#{Tower}::util::BoxCloneService::new(svc), $requestSpecVarName)
""",
"RequestFmt" to sensitivityGen.requestFmt().value,
Expand Down
32 changes: 16 additions & 16 deletions ...amazon/smithy/rust/codegen/server/smithy/generators/ServerHttpSensitivityGeneratorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class ServerHttpSensitivityGeneratorTest {
rustTemplate(
"""
let closure = #{Closure:W};
assert_eq!(closure("query_a"), #{SmithyHttpServer}::logging::sensitivity::uri::QueryMarker { key: false, value: false });
assert_eq!(closure("query_b"), #{SmithyHttpServer}::logging::sensitivity::uri::QueryMarker { key: false, value: true });
assert_eq!(closure("query_a"), #{SmithyHttpServer}::instrumentation::sensitivity::uri::QueryMarker { key: false, value: false });
assert_eq!(closure("query_b"), #{SmithyHttpServer}::instrumentation::sensitivity::uri::QueryMarker { key: false, value: true });
""",
"Closure" to querySensitivity.closure(),
*codegenScope,
Expand Down Expand Up @@ -110,7 +110,7 @@ class ServerHttpSensitivityGeneratorTest {
rustTemplate(
"""
let closure = #{Closure:W};
assert_eq!(closure("wildcard"), #{SmithyHttpServer}::logging::sensitivity::uri::QueryMarker { key: true, value: true });
assert_eq!(closure("wildcard"), #{SmithyHttpServer}::instrumentation::sensitivity::uri::QueryMarker { key: true, value: true });
""",
"Closure" to querySensitivity.closure(),
*codegenScope,
Expand Down Expand Up @@ -158,7 +158,7 @@ class ServerHttpSensitivityGeneratorTest {
rustTemplate(
"""
let closure = #{Closure:W};
assert_eq!(closure("wildcard"), #{SmithyHttpServer}::logging::sensitivity::uri::QueryMarker { key: true, value: false });
assert_eq!(closure("wildcard"), #{SmithyHttpServer}::instrumentation::sensitivity::uri::QueryMarker { key: true, value: false });
""",
"Closure" to querySensitivity.closure(),
*codegenScope,
Expand Down Expand Up @@ -206,7 +206,7 @@ class ServerHttpSensitivityGeneratorTest {
rustTemplate(
"""
let closure = #{Closure:W};
assert_eq!(closure("wildcard"), #{SmithyHttpServer}::logging::sensitivity::uri::QueryMarker { key: false, value: true });
assert_eq!(closure("wildcard"), #{SmithyHttpServer}::instrumentation::sensitivity::uri::QueryMarker { key: false, value: true });
""",
"Closure" to querySensitivity.closure(),
*codegenScope,
Expand Down Expand Up @@ -284,9 +284,9 @@ class ServerHttpSensitivityGeneratorTest {
"""
let closure = #{Closure:W};
let name = #{Http}::header::HeaderName::from_static("header-a");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
let name = #{Http}::header::HeaderName::from_static("header-b");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: true, key_suffix: None });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: true, key_suffix: None });
""",
"Closure" to headerData.closure(),
*codegenScope,
Expand Down Expand Up @@ -332,11 +332,11 @@ class ServerHttpSensitivityGeneratorTest {
"""
let closure = #{Closure:W};
let name = #{Http}::header::HeaderName::from_static("prefix-a");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: true, key_suffix: Some(7) });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: true, key_suffix: Some(7) });
let name = #{Http}::header::HeaderName::from_static("prefix-b");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: true, key_suffix: Some(7) });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: true, key_suffix: Some(7) });
let name = #{Http}::header::HeaderName::from_static("other");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
""",
"Closure" to headerData.closure(),
*codegenScope,
Expand Down Expand Up @@ -415,11 +415,11 @@ class ServerHttpSensitivityGeneratorTest {
"""
let closure = #{Closure:W};
let name = #{Http}::header::HeaderName::from_static("prefix-a");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: false, key_suffix: Some(7) });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: false, key_suffix: Some(7) });
let name = #{Http}::header::HeaderName::from_static("prefix-b");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: false, key_suffix: Some(7) });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: false, key_suffix: Some(7) });
let name = #{Http}::header::HeaderName::from_static("other");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
""",
"Closure" to headerData.closure(),
*codegenScope,
Expand Down Expand Up @@ -469,11 +469,11 @@ class ServerHttpSensitivityGeneratorTest {
"""
let closure = #{Closure:W};
let name = #{Http}::header::HeaderName::from_static("prefix-a");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: true, key_suffix: None });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: true, key_suffix: None });
let name = #{Http}::header::HeaderName::from_static("prefix-b");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: true, key_suffix: None });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: true, key_suffix: None });
let name = #{Http}::header::HeaderName::from_static("other");
assert_eq!(closure(&name), #{SmithyHttpServer}::logging::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
assert_eq!(closure(&name), #{SmithyHttpServer}::instrumentation::sensitivity::headers::HeaderMarker { value: false, key_suffix: None });
""",
"Closure" to headerData.closure(),
*codegenScope,
Expand Down
Loading

0 comments on commit dcfb855

Please sign in to comment.