Add new "--log-to-stdout" CLI flag

[Kami]

This pull request adds new --log-to-stdout flag to the agent which can be used in combination with the existing --no-fork flag.

When this flag is used, the agent will log all the messages to stdout.

This comes handy in various scenarios such as when troubleshooting things and for local development.

In builds on top of my comments from #415.

TODO

• Tests

[codecov]

Codecov Report

Merging #460 into master will increase coverage by 0.04%.
The diff coverage is 91.67%.

@@            Coverage Diff             @@
##           master     #460      +/-   ##
==========================================
+ Coverage   67.88%   67.92%   +0.04%     
==========================================
  Files         107      107              
  Lines       26981    27012      +31     
  Branches     3284     3283       -1     
==========================================
+ Hits        18314    18347      +33     
  Misses       7797     7797              
+ Partials      870      868       -2

Impacted Files	Coverage Δ
scalyr_agent/scalyr_logging.py	80.58% <ø> (+0.58%)	⬆️
scalyr_agent/tests/agent_main_test.py	100% <100%> (ø)	⬆️
scalyr_agent/agent_main.py	49.08% <57.14%> (+0.08%)	⬆️
.../builtin_monitors/tests/kubernetes_monitor_test.py	96.7% <0%> (ø)	⬆️
scalyr_agent/util.py	85.54% <0%> (+0.14%)	⬆️

[Kami]

@czerwingithub What's your take on this?

I think it's reasonable and something I missed from the agent.

Most of the Unix daemons which have foreground / no-fork mode usually also have an option to log things to stdout instead of log files.

Usually that's also the default behavior when "foreground" mode is used, but I know this really won't work for us because our agent relies on logs being written to disk so we can ship them to Scalyr API.

[czerwingithub]

I hear what you are saying about the expected behavior of non-forked processes. Maybe we should strive to respect that. So, I have a little different take on this. Let me know what you think.

1. When non-forked, log (by default) to both stdout and the agent log. You are right that we do still want that to go up to Scalyr.

2. We then need to preserve the current behavior that K8s and Docker does not have the agent log go to stdout (even though they are not forked.) So, we can have a special --full-log-to-stdout flag that K8s and Docker can specify as false in their run commands. That will be documented as, if true, then send the full agent.log to stdout if the process is not forked.

I know #2 is a little odd, but only we'd have to really use it when defining the container images we publish.

Thoughts?

[yanscalyr]

One idea I had is to avoid any new flags at all we could use the "log only things above some severity to stdout" functionality in place of it. By default we could have no-fork processes log INFO and above, then in the k8s config file define it as ERROR or something else that would only output things that would be useful to see in k8s logs. The documentations for the config file would also advertise this new feature that some customers wanted.

A concern I have with this idea is that people upgrading their k8s agents wouldn't update their config and would get everything logged to stdout.

[czerwingithub]

I think you might be onto something there Yan. I like that idea and I'll add one new twist to it.. but I don't know if it will work.

It is possible in Dockerfiles to define values for environment variables. If we define an ENV variable for our K8s container images, will that be used if the user has not explicitly set a value for that variable in the configmap? If so, then we don't need to set it in the K8s config but instead in the actual container images.. so we won't have that config divergence problem you alluded to.

[yanscalyr]

That actually sounds perfect! I will let @Kami comment on it as well but I think thats the way to go.

[czerwingithub]

@yanscalyr You might want to make sure it works... an ENV variable defined in your container is available in K8s... and that you can override it by defining variables in the K8s config. You could do a small experiment and see.

[Kami]

@yanscalyr That approach sounds good to me 👍

And ideally, for development / testing purposes you could also control this level / severity setting.

[Kami]

@yanscalyr How do you want me to proceed with this PR?

Should I close it? Should we merge it? Will you incorporate it into your changes?

[yanscalyr]

@Kami probably just close this PR, the work is pretty duplicate with mine and mine is also almost ready.
@czerwingithub Did some testing and it does seem to work, so we would default the severity limit to DEBUG (I need to test that the debug level config option will stop actual debug messages unless we set it, shouldn't be hard to add if it doesn't just work) and in the k8s docker file set the environment variable to WARN I guess, unless you think ERROR would be better.