Do not log small request bodies

[arybczak]

They might contain sensitive information.

[endgame]

I have thought about this for a few days, and I'm still not sure what the best action is here. It's extremely useful to see the smaller request bodies when debugging, and because it's done with typeclasses we kinda have to commit to one or the other.

OTOH, you don't have to use the built-in logging; 2.0 implements it using the hooks system. You could implement your own hooks which log request bodies differently.

Also, request bodies are only logged at the Trace log level, so if you don't need to see response bodies you could log at a different level?

In the long term, I would rather Amazonka didn't have any baked-in logging at all, and it became a separate client using the hooks system: #753 . The reasons I've not done that yet are:

• It would've held up the 2.0 release, and
• Some downstream libraries, like amazonka-s3-streaming currently piggy-back off of it. (Ping @axman6 — have you thought about how you might log if I removed logging from package amazonka?)

I'm interested in people's thoughts here to find a better way forward. An intermediate step might be to stop relying on class ToLog, so that we can more easily substitute different levels of detail.

[axman6]

My logging is dumb and should probably be removed 🙃

[arybczak]

OTOH, you don't have to use the built-in logging; 2.0 implements it using the hooks system. You could implement your own hooks which log request bodies differently.

Thanks for the pointer, I didn't know that. I replaced default hooks with custom ones that log exactly what's needed :+1: So in principle I don't need this PR anymore.

Also, request bodies are only logged at the Trace log level, so if you don't need to see response bodies you could log at a different level?

Response body is logged at the Trace level, request body is logged at the Debug level.

FYI, the main issue with the current state I found is that you can send a request constructed with e.g. newPutObject bucket url (toBody $ Sensitive fileContents) and fileContents will still get logged if it's less than 4KB, because the Sensitive newtype is discarded during internal type conversions.

[endgame]

Response body is logged at the Trace level, request body is logged at the Debug level.

Should we change this? What if we logged that a request was made at Debug, and only logged (full, small) request/response bodies at Trace?

As for removing the idea of a "logger" from the Amazonka Env type: what is an acceptable interface here? I worry that silencing Amazonka on an upgrade isn't a great experience for people, and maybe we still want to provide the logging hooks to give people a reasonable set of default messages?

[arybczak]

What if we logged that a request was made at Debug, and only logged (full, small) request/response bodies at Trace?

Indeed, that would be better than the current situation :slightly_smiling_face:

As for removing the idea of a "logger" from the Amazonka Env type: what is an acceptable interface here? I worry that silencing Amazonka on an upgrade isn't a great experience for people, and maybe we still want to provide the logging hooks to give people a reasonable set of default messages?

To be fair, now that I learned about hooks, I think the current interface is good. You get reasonable defaults that can be fully customized if you want to / have special needs.