[
https://issues.apache.org/jira/browse/FLUME-828?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217780#comment-13217780
]
[email protected] commented on FLUME-828:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3928/#review5375
-----------------------------------------------------------
Thanks for the patch Brock. Some comments follow:
* Please remove any trailing whitespaces from the code where present. These are
highlighted in red on the review board.
flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
<https://reviews.apache.org/r/3928/#comment11704>
Need to guard against a null body.
flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
<https://reviews.apache.org/r/3928/#comment11706>
This is the same variable name as what is used on line 80. Please use a
different name for avoiding confusion.
Also, ideally, the character encoding should be read from a system header
value within the event. If the header is not present, the default encoding of
UTF-8 could be used. That way the representation stays flexible.
flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
<https://reviews.apache.org/r/3928/#comment11705>
would be good to log this exception as well.
- Arvind
On 2012-02-17 19:51:58, Brock Noland wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3928/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-02-17 19:51:58)
bq.
bq.
bq. Review request for Flume.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Changes SimpleEvent.toString() to include a human readable representation
of the body byte array.
bq.
bq.
bq. This addresses bug FLUME-828.
bq. https://issues.apache.org/jira/browse/FLUME-828
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. flume-ng-core/src/main/java/org/apache/flume/event/SimpleEvent.java
e0c3b45
bq. flume-ng-core/src/test/java/org/apache/flume/event/TestSimpleEvent.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/3928/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Added two tests for the new code.
bq.
bq.
bq. Thanks,
bq.
bq. Brock
bq.
bq.
> LoggerSink representation of the event's body isn't too useful
> --------------------------------------------------------------
>
> Key: FLUME-828
> URL: https://issues.apache.org/jira/browse/FLUME-828
> Project: Flume
> Issue Type: Improvement
> Components: Sinks+Sources
> Affects Versions: NG alpha 1
> Reporter: Will McQueen
> Assignee: Brock Noland
> Fix For: v1.1.0
>
> Attachments: FLUME-828-0.patch, FLUME-828-1.patch
>
>
> LoggerSink logs entries to console that looks like this:
> Event: { headers:{} body:[B@5c1ae90c }
> ...where the body is just "getClass().getName() + "@" +
> Integer.toHexString(hashCode())". The "getClass().getName() will always
> resolve to [B.
> The issue seems to be how can we represent a SimpleEvent's payload as a
> String, when the payload is some arbitrary byte array... the array's bytes
> could represent encoded ascii chars, encoded UTF-8 chars, or binary data such
> as an encrypted payload. If we default to ASCII translation for everything,
> then the resulting String won't be useful for binary payloads since not all
> 256 possible bytes have equivalent printable ASCII chars. Here's one idea:
> For each event body, we can print up to the first 16 bytes in hex format. If
> there are >16 bytes, then print a "..." suffix at the end. The output would
> look similar to what you get with unix "hexdump -C". Here's what a sample
> output from LoggerSink would look like:
> Event: { headers:{} body: 00000000 54 68 65 20 71 75 69 63 6B 20 62 72
> 6F 77 6E 20 |The quick brown | ... }
> ...where both the hex and the ascii are displayed for the first 16 chars.
> Is it the most useful representation of the body? Probably not. Is it as
> least more useful than printing "[B@" + Integer.toHexString(hashCode())"? I
> think so.
> The commons io lib has a useful HexDump.dump cmd we can leverage.
> Thoughts?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
