Ability to configure LogginerEmitter to only emit metrics or only alerts or both #17
Conversation
| } | ||
| break; | ||
| default: | ||
| throw new ISE("unknown log events eventsToLog [%s].", eventsToLog); |
There was a problem hiding this comment.
"eventsToLog set to [%s], possible values are ALL, ALERTS or METRICS " ?
himanshug
force-pushed
the
logging_emitter_choice
branch
from
c272581
to
1026a38
Compare
|
There will be a new property druid.emitter.logging.eventsToLog in druid.io configuration.md? |
|
@rasahner yes, will add that once this PR is merged. |
|
@drcrallen @xvrl @nishantmonu51 can one of you pls review/merge this one? |
| import java.util.Map; | ||
| import java.util.Properties; | ||
|
|
||
| public class Emitters |
There was a problem hiding this comment.
any reason we are removing this class?
There was a problem hiding this comment.
its not needed anymore, it was used by unit tests and was doing unnecessary conversions. pls see the updated unit tests.
There was a problem hiding this comment.
we still use this class internally in other projects, so we cannot remove it
There was a problem hiding this comment.
oh, ok, let me add it back then.
There was a problem hiding this comment.
@xvrl added it back.
himanshug
force-pushed
the
logging_emitter_choice
branch
from
1026a38
to
f1e3aaf
Compare
|
@xvrl pls merge if there are no further comments. |
| final ObjectMapper objectMapper = new ObjectMapper(); | ||
| final HttpEmitterConfig config = objectMapper.convertValue(Emitters.makeHttpMap(props), HttpEmitterConfig.class); |
There was a problem hiding this comment.
looks like we are removing testing of Emitters.makeHttpMap here.
There was a problem hiding this comment.
that is true.
does the other code you have that creates object doesn't do it the way druid does and instead relies on the property transformations in Emitters.makeHttpMap?
|
@himanshug sorry this sat for a bit, is it still needed? |
|
@drcrallen we really wanted to use logging emitter to print alerts in logs but it appears they are always printed as error messages in the logs , so there wasn't a major need. however it will be nice to have this capacity at some point. |
most of the monitoring systems can't handle the stack-traces druid alerts contain, however they contain useful information.. we want the ability to push metrics to other systems and yet be able to log just the alerts in the process logs. This PR combined with composing emitter enables doing that with other emitters such as http post.
also, a bit of cleanup in Logging/Http EmitterConfigTest