Logging failure after 1.1

[tgross]

Given the following config file:

{
  "consul": "consul:8500",
  "logging": {
    "level": "DEBUG",
    "format": "default",
    "output": "stderr"
  },
  "services": [
    {
      "name": "myservice",
      "port": 80,
      "poll": 10,
      "ttl": 30
    }
  ]
}

we expect the output to be:

$ ./containerbuddy -config file:///containerbuddy.json hello-world.sh
2016/03/09 19:16:18 `health` is required in service jenkins

but instead we get:

$ ./containerbuddy -config file:///containerbuddy.json hello-world.sh
$ echo $?
1

This failure happens after the logging framework is set up in this section. But if we force the failure to happen before we set up the logging, for example with the config file, we get the same non-logging behavior:

{
  "consul": "consul:8500",
  "etcd": "etcd:4001",
  "logging": {
    "level": "DEBUG",
    "format": "default",
    "output": "stderr"
  },
  "services": [
    {
      "name": "myservice",
      "port": 80,
      "poll": 10,
      "ttl": 30
    }
  ]
}

[tgross]

As an aside, and we should fix it here, the logging config in the example in the README still says "format": "go" instead of "format": "default".

[tgross]

Removing the init seems to fix this but I'm not sure I understand why yet.

[tgross]

It looks to me from the logrus docs that when we're importing logrus via log "github.com/Sirupsen/logrus" we're getting Logrus' default logging anyways. PR inbound.

[tgross]

PR inbound.

Nope! Turns out I was totally off-base there. It's our default logger that breaks everything. Works fine if we pass text or json as the output format. That narrows things quite a bit!

[tgross]

I have a minimal failing test case that exercises the broken code:

func TestDefaultFormatter(t *testing.T) {
    formatter := &DefaultLogFormatter{}
    b, err := formatter.Format(log.WithFields(
        log.Fields{
            "level": "info",
            "msg":   "something",
        },
    ))
    fmt.Printf("%v\n", err)
    fmt.Println(string(b))
}

This test panics(!) because it reaches log.Panicln, which we shouldn't be hitting at all!

I'm still working thru this but will pick it up tomorrow morning. @justenwalker any thoughts on this?

[justenwalker]

I think the problem is here

Essentially, PanicLevel == 0, so any log entry with an un-initialized level is assumed to be Panic

[justenwalker]

The real issue is handling Panic and Fatal in the Formatter - it just doesn't make sense and it is also redundant since Logrus does this for us. #106 should fix the bug (and update the README for the "go" format)

[tgross]

Thanks for the assist, @justenwalker!

[tgross]

I've verified that this fix corrects the problem described at the top of the issue as well.

[tgross]

Will release this in 1.2.0 (master has a new config flag from #102) this morning.

[tgross]

Released in https://github.com/joyent/containerbuddy/releases/tag/1.2.0