fn icon indicating copy to clipboard operation
fn copied to clipboard

default user logs to send to fn stderr at INFO level

Open rdallman opened this issue 5 years ago • 0 comments

closes #1474

see linked issue above for some details of the saga. this patch does a number of things in order to accomplish the goal of sending user logs to fn stderr at INFO level, while allowing it to easily be configured to off. as pure_runner is explicitly setting these to off for the moment, that configuration is not affected by this patch, however in the future it's possible we may want to change these to align so that lb/runner configs can get logs out of user containers in this manner. this only affects the full node (not just full agent) configuration, where calls are created in the invoke/httptrigger handlers from OSS fn. open to discussion on default behavior, however I think we have to have logs by default in some capacity for OSS, absolutely want to make it not brittle and easily disabled for production services. attempt to cover surface area here:

  • moves the logger configuration up to a CallOpt instead of getting defaulted to in GetCall, now GetCall defaults to configuring logs to off, which seems much less brittle for other packages that call into agent and may absolutely not want logs (they exist). now our handlers in OSS configure this setting. let the record show, I'm not perfectly content with some of the munging that had to go on here (there's agent env vars to configure this option which is called from the front end, which needs the config in call opts), however after trying a few things this seems the most reasonable but I am very much open to concrete ideas - this was a particular pain point.
  • changed stderr to io.WriteCloser as there's no longer a need to buffer logs since we are not uploading them anywhere, modifies our func logger heavily to remove all the old cruft and adds the ability to configure log writer with a level
  • adds ability to configure FN_USER_LOG_LEVEL (i'll update docs) which defaults to 'info'. the level at which this is defaulted to info is open for discussion. removes unused FN_MAX_LOG_SIZE_BYTES
  • updates logrus to 1.4.1 for logger.Log(level, stuff) method
  • deleted a lot of old TODOs from yours truly
  • deleted StdErr method from RunnerCall interface, as far as I can tell in all our repos here and in the service, this method is uncalled (can remove at will, since any implementers it's unused and still satisfies the interface)
  • fixes #1328 issue by using buffer.ReadBytes('\n')
  • fixed bug where we'd log between containers' logs at debug level when logs are configured off
  • fixes a couple tests that fail for me now on docker 18.09.5 w/ new 404 message

tested and she seems to work as expected

rdallman avatar Apr 19 '19 20:04 rdallman