search

weaveworks / launcher

Weave Cloud Launcher
Apache License 2.0
10 stars 13 forks source link

Small refactorings and leftover stuff from previous PR #230

Closed lilic closed 6 years ago

lilic commented 6 years ago

This just extracts into a separate cloudwatch.go file and also separates the tests as previously discussed with @dlespiau. And addresses minor logging comments that @marccarre left in the previous PR.

Not entirely certain how much we want to log, if at all the msgs about each time an event is triggered are those useful? Any suggestions welcome!

Feel like below is too much noise maybe:

time="2018-06-12T10:24:39Z" level=info msg="configmaps \"cloudwatch\" not found"
time="2018-06-12T10:25:38Z" level=info msg="Secret was created"
time="2018-06-12T10:25:38Z" level=info msg="configmaps \"cloudwatch\" not found"
time="2018-06-12T10:26:06Z" level=info msg="ConfigMap was created"
time="2018-06-12T10:26:06Z" level=error msg="cloudwatch: at least one AWS resource must be specified"
time="2018-06-12T11:19:22Z" level=info msg="ConfigMap was updated"
marccarre commented 6 years ago

Feel like below is too much noise maybe

I haven't debugged the launcher enough to be able to tell whether it is too noisy or not, sorry :slightly_smiling_face: However, I find the current logs harder to "parse" than, for example:

level=info msg="ConfigMap [weave/configmaps] not found"
level=info msg="Secret [weave/secrets] created"
level=info msg="ConfigMap [weave/configmaps] not found"
level=info msg="ConfigMap [weave/configmaps] created"
level=error msg="ConfigMap [weave/configmaps/cloudwatch] should specify at least one AWS resource"
level=info msg="ConfigMap [weave/configmaps] updated"

or

level=info msg="ConfigMap not found" ns="weave" name="configmaps"
level=info msg="Secret created" ns="weave" name="secrets"
level=info msg="ConfigMap not found" ns="weave" name="configmaps"
level=info msg="ConfigMap created" ns="weave" name="configmaps"
level=error msg="ConfigMap should specify at least one AWS resource" ns="weave" name="configmaps" entry="cloudwatch"
level=info msg="ConfigMap updated" ns="weave" name="configmaps"

</nitpick>

dlespiau commented 6 years ago

Maybe a good way to make a difference between Info and Debug messages is whether we are doing something with the new Secret/CM or not. ie. detecting a new secret should be a Debug message, finding a valid cloudwatch secret/cm and the download URL should be Info messages.

lilic commented 6 years ago

Done, logs make more sense now. 👍 PTALA, thanks!

dlespiau commented 6 years ago

lgtm!