Add support for Extended Log Format #1304
Conversation
greg65236592
requested review from
whiskeysierra,
lukasniemeier-zalando and
AlexanderYastrebov
as code owners
logbook-core/src/main/java/org/zalando/logbook/ExtendedLogFormatSink.java
Outdated
Show resolved
Hide resolved
logbook-core/src/main/java/org/zalando/logbook/ExtendedLogFormatSink.java
Outdated
Show resolved
Hide resolved
logbook-core/src/main/java/org/zalando/logbook/ExtendedLogFormatSink.java
Outdated
Show resolved
Hide resolved
logbook-core/src/main/java/org/zalando/logbook/ExtendedLogFormatSink.java
Show resolved
Hide resolved
|
|
||
| class RemoteResponseTest { | ||
|
|
||
| @Test | ||
| void statusCanThrow() throws IOException { |
There was a problem hiding this comment.
Any particular reason why this test was removed?
There was a problem hiding this comment.
It was removed due to the dependency upgrade. I have reverted this commit at 2a16738 to see if it can pass the build.
logbook-core/src/main/java/org/zalando/logbook/ExtendedLogFormatSink.java
Outdated
Show resolved
Hide resolved
| * @see <a href="https://docs.oracle.com/cd/A97329_03/bi.902/a90500/admin-05.htm#634823">Oracle analytical tool log | ||
| * parsing description</a> | ||
| */ | ||
| private enum Field { |
There was a problem hiding this comment.
It would be nicer if this enum not only holds the field name, but also the logic (as a lambda) how to extract the information. That way the field name and its implementation would be very close to each other. It would also remove the need for the lcoal variables with the comments above.
There was a problem hiding this comment.
Nice suggestion. Since the logic of getting field values requires data from several places, such as request and response, I have to encapsulate those objects into FieldParameter for the lambda usage, please see if that fits. 4543589
This reverts commit 944df6f.
remove unnecessary comments and add final to variables
|
Thanks for reviewing! All comments should be addressed now. |
|
Looks good to me 👍🏼 Let's just fix the build, and we should be able to merge it now |
This relates to jeremylong/DependencyCheck#5502.
|
Looks like the issue: jeremylong/DependencyCheck#5502 is not resolved yet, add a suppression to pass the build. |
|
👍🏼 |
Add support for Extended Log Format(ELF)
Description
Adding support for Extended Log Format(ELF).
From Wikipedia: ELF is a standardised text file format, like Common Log Format (CLF), that is used by web servers when generating log files, but ELF files provide more information and flexibility.
ELF is also described in W3C.
Motivation and Context
Logbook usually separates the logging of request and response like described, except for Common Log Format.
In some cases, we might want use tools to query or analyze server logs, and it could be much easier to get it done if we could print the request and response log in one row.
CommonsLogFormatSinkis the only one that can do this among all sinks. If we like to print other fields like request time taken, or some special header values, CLF is unable to provide such information.Thus, I would like to bring in Extended Log Format(ELF) for extending the CLF, which allows users to customize log fields, and outputs the request and response information together in the same row.
Types of changes
Checklist: