evalphobia / logrus_fluent

fluentd hooks for logrus
Apache License 2.0
45 stars 22 forks source link

Why is NewHook deprecated? #18

Closed sagikazarmark closed 7 years ago

sagikazarmark commented 7 years ago

I understand that it would mean that by default the hook would open a new connection every time and it's not desired. What I specifically miss is the ability to open an Async connection to fluent which I can only do if I manually set the fluent instance in the hook.

So can we depend on NewHook in the future or should I use New instead and fallback to an async connection when the first connection fails?

(Context: I use docker-compose where the order of startup is not guaranteed, so in theory fluentd might not start before my app, but I don't want my app to fail because of missing fluentd connection).

Any help or thoughts would be appreciated!

Thanks in advance!

evalphobia commented 7 years ago

@sagikazarmark Sorry for my late reply.

I'll remove NewHook in the future, but not near future. Instead, I'll add NewWithConfig feature and you can choose behavior in that function and config.

It will include NewHook's connection and official library config. (It seems to have AsyncConnect and MaxRetry for that purpose)

evalphobia commented 7 years ago

@sagikazarmark Since v0.4.0, now you can use NewWithConfig and Config.DisableConnectionPool = true for your purpose.

hook, err := logrus_fluent.NewWithConfig(logrus_fluent.Config{
    Host:                  "localhost",
    Port:                  24224,
    DisableConnectionPool: true,
})
sagikazarmark commented 7 years ago

Great, thanks