-
Notifications
You must be signed in to change notification settings - Fork 487
Kafka log listener #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kafka log listener #178
Conversation
| map.put(prefix + c.getKey(), ((ISOField)c).getValue()); | ||
| }else { | ||
| Map field = new LinkedHashMap(); | ||
| if((((ISOBasePackager) packager).getFieldPackager((int) c.getKey()).getClass().getName()).contains("NUMERIC")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're assuming ISOBasePackager and this could throw a class cast exception.
| map.put(prefix + c.getKey(), ((ISOField)c).getValue()); | ||
| }else { | ||
| Map field = new LinkedHashMap(); | ||
| if((((ISOBasePackager) packager).getFieldPackager((int) c.getKey()).getClass().getName()).contains("NUMERIC")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't go by name containing NUMERIC to determine the type - need something better - isn't there some packager patterns JPOS uses already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Vishu, I didn't merge this PR just yet because I'm not sure which JSON dependency we want to add. In jPOS-EE we are using Jackson, I believe this one uses JSON Simple (kinda dead), we have some others using GSON (kinda dead). @vsalaman suggested https://bolerio.github.io/mjson which looks very nice. So we'll see which one to use in jPOS, and then rework this PR with @andresantoniuk
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ar thanks for the note. I didn't know GSON is considered obsolete. I see commits in Aug 2018. I haven't used mjson but will try it out some time.
| <property name="name" value="logger.Q2.buffered" /> | ||
| </log-listener> | ||
|
|
||
| <log-listener class="org.jpos.util.KafkaLogListener"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, this logger file should not add Kafka listener - perhaps provide this as an example elsewhere?
|
Great PR, but I think it's better suited as a jPOS-EE module |
This is a preliminary version of a new LogListener this goals:
Configuration is done via a new log-listener section on
00_logger.xmlthat forwards configuration options to the Kafka Producer APITesting environment
A nice view from kibana here
Notes:
Originally I plan to add this LogListener as a new JPOS-EE module, but as JSON is needed I consider better to enable JSON packager that was on hold here.
Need to continue working on LogEvents that do not contain payload, specially on
TransactionManagerThis is part of my journey with jPOS and Elastic for monitoring and transactional data analysis.