richardwilly98 / elasticsearch-river-mongodb

MongoDB River Plugin for ElasticSearch
1.12k stars 215 forks source link

Use loggers provided by AbstractRiverComponent, prefixed with river name #521

Closed kdkeck closed 9 years ago

kdkeck commented 9 years ago

When you're running more than one river, it's very helpful if all log messages specific to a particular river include the river name. Extending AbstractRiverComponent automatically creates a logger which does that, and more.

benmccann commented 9 years ago

I like the idea of including the river name in the log message. Maybe we could do it a bit differently though

It seems like this would cause it to be such that there's only a single logger? So you could no longer change the log level for a single class. And it would no longer print the classname in the log message.

Also, not sure if you saw this, but they've deprecated rivers, so we're going to have to make this a standalone binary in the future. So it might be better not to depend on AbstractRiverComponent given that.

kdkeck commented 9 years ago

No, there's not only one logger, there's instead one for each instance of each component class, and you can still set different log levels for each class. For some reason the ES loggers leave the class name out of the first prefix when the package is under org.elasticsearch, but they are still separate loggers.

We could implement our own mechanism, but that seems silly when there's one already provided. There's really not much to AbstractRiverComponent, so I don't see a big issue with getting tied to it. And presumably as other rivers are converted into standalone binaries, they'll be replicating these services they previously depended on, and I would really hope that could be done as a module that all rivers could use in the conversion.

benmccann commented 9 years ago

I think it's a bit weird it leaves the class name out. I'll go ahead and merge this for now and we can update later if it turns out to be a problem