Skip to content
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

Add server side HTTP logging layer #1550

Merged
merged 72 commits into from
Aug 2, 2022
Merged

Add server side HTTP logging layer #1550

merged 72 commits into from
Aug 2, 2022

Conversation

hlbarber
Copy link
Contributor

@hlbarber hlbarber commented Jul 14, 2022

Motivation and Context

#1536

Description

  • Add MakeFmt trait which modifies Display/Debug implementations.
  • Add http wrappers which modify the Debug and Display implementations based on closures marking sensitivity. Each wrapper has an associated MakeFmt impl.
  • Add InstrumentOperation tower::Service which logs requests and responses. Accepts a MakeFmt for each potentially sensitive component of the request/response.
  • Add ServerHttpSensitivityGenerator.kt which generates marker closures from models.
  • Apply InstrumentationOperation to the OperationHandler using MakeFmts using the marker closures.

@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 15, 2022
@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 15, 2022
@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 15, 2022
@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 15, 2022
@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 27, 2022
@smithy-lang smithy-lang deleted a comment from github-actions bot Jul 27, 2022
@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

)

// Models the ways headers can be bound and sensitive
sealed class HeaderSensitivity(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does bound mean in this context?

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@github-actions
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link
Contributor

@crisidev crisidev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small comment about some missing documentation in the model. LGTM.

codegen-server-test/model/pokemon.smithy Outdated Show resolved Hide resolved
Comment on lines +98 to +100
fn make_display(&self, source: T) -> Self::Target {
U::make(self, source)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to leave it like it is. It was NIT anyway :)

@hlbarber hlbarber requested a review from a team as a code owner August 2, 2022 20:43
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

A new generated diff is ready to view.

A new doc preview is ready to view.

@hlbarber hlbarber enabled auto-merge (squash) August 2, 2022 21:28
@hlbarber hlbarber merged commit ed2a866 into main Aug 2, 2022
@hlbarber hlbarber deleted the harryb/add-logging branch August 2, 2022 21:32
@crisidev
Copy link
Contributor

crisidev commented Aug 3, 2022

❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants