cloudfoundry / eirini-release

Helm release for Project Eirini
Apache License 2.0
36 stars 34 forks source link

Improve logs for `PUT /apps/:guid/:vguid/stop/:id` #194

Open gcapizzi opened 3 years ago

gcapizzi commented 3 years ago

This is how logs look like when stopping an LRP instance:

{"timestamp":"...","level":"debug","source":"handler","message":"handler.stop-app-instance.requested","data":{"guid":"...","session":"13","version":"..."}}

It's not very clear what handler means, and we don't see the start and end of the request processing, which could be useful especially once we introduce extra debug logs or just to see request duration. Also, this should probably be an info log. So something like this:

{"timestamp":"...","level":"info","source":"api","message":"stop-app-instance.started","data":{"session":"218", "guid": "..."}}
{"timestamp":"...","level":"info","source":"api","message":"stop-app-instance.listing-stateful-sets","data":{"session":"218", "label-selector": "..."}}
{"timestamp":"...","level":"info","source":"api","message":"stop-app-instance.deleting-pod","data":{"session":"218", ...}}
{"timestamp":"...","level":"info","source":"api","message":"stop-app-instance.ended","data":{"session":"218", "response-code": 200}}

Let's also make sure that any errors are logged correctly and only once. Refer to #177562455 for details about how we want to log errors.

Dev Notes

Ensure logs are correlated using lager session IDs. This requires using the logger.Session method and passing the logger through method calls. It will not work if the logger is stored in a struct.

Logger can be passed as an explicit method argument, or might also be passed as part of the context object that is already passed around (see the https://github.com/cloudfoundry/lager/tree/master/lagerctx package, and note in this story). With lagerctx, there is the risk of accidentally and unknowingly using the fallback discard logger if the original context is somehow lost while passing through methods. So explicitly passing both context and logger as separate method parameters throughout might be the safest option, giving compile-time guarantees of having logging correctly propagated.