Skip to content

Commit cbbd820

Browse files
cekiceki
committed
fix issues/866
Signed-off-by: Ceki Gulcu <[email protected]>
1 parent 77e95e0 commit cbbd820

File tree

2 files changed

+58
-28
lines changed

2 files changed

+58
-28
lines changed

‎logback-classic/src/main/java/ch/qos/logback/classic/encoder/JsonEncoder.java

+40-25
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.slf4j.Marker;
2424
import org.slf4j.event.KeyValuePair;
2525

26+
import java.util.Arrays;
2627
import java.util.List;
2728
import java.util.Map;
2829
import java.util.Set;
@@ -108,7 +109,6 @@ public class JsonEncoder extends EncoderBase<ILoggingEvent> {
108109
private boolean withTimestamp = true;
109110
private boolean withNanoseconds = true;
110111

111-
112112
private boolean withLevel = true;
113113
private boolean withThreadName = true;
114114
private boolean withLoggerName = true;
@@ -121,7 +121,6 @@ public class JsonEncoder extends EncoderBase<ILoggingEvent> {
121121
private boolean withThrowable = true;
122122
private boolean withFormattedMessage = false;
123123

124-
125124
@Override
126125
public byte[] headerBytes() {
127126
return EMPTY_BYTES;
@@ -135,39 +134,40 @@ public byte[] encode(ILoggingEvent event) {
135134

136135
if (withSequenceNumber) {
137136
appenderMemberWithLongValue(sb, SEQUENCE_NUMBER_ATTR_NAME, event.getSequenceNumber());
138-
sb.append(VALUE_SEPARATOR);
139137
}
140138

141139
if (withTimestamp) {
140+
appendValueSeparator(sb, withSequenceNumber);
142141
appenderMemberWithLongValue(sb, TIMESTAMP_ATTR_NAME, event.getTimeStamp());
143-
sb.append(VALUE_SEPARATOR);
144142
}
145143

146144
if (withNanoseconds) {
145+
appendValueSeparator(sb, withSequenceNumber, withTimestamp);
147146
appenderMemberWithLongValue(sb, NANOSECONDS_ATTR_NAME, event.getNanoseconds());
148-
sb.append(VALUE_SEPARATOR);
149147
}
150148

151149
if (withLevel) {
150+
appendValueSeparator(sb, withNanoseconds, withSequenceNumber, withTimestamp);
152151
String levelStr = event.getLevel() != null ? event.getLevel().levelStr : NULL_STR;
153152
appenderMember(sb, LEVEL_ATTR_NAME, levelStr);
154-
sb.append(VALUE_SEPARATOR);
155153
}
156154

157155
if (withThreadName) {
156+
appendValueSeparator(sb, withLevel, withNanoseconds, withSequenceNumber, withTimestamp);
158157
appenderMember(sb, THREAD_NAME_ATTR_NAME, jsonEscape(event.getThreadName()));
159-
sb.append(VALUE_SEPARATOR);
160158
}
161159

162160
if (withLoggerName) {
161+
appendValueSeparator(sb, withThreadName, withLevel, withNanoseconds, withSequenceNumber, withTimestamp);
163162
appenderMember(sb, LOGGER_ATTR_NAME, event.getLoggerName());
164-
sb.append(VALUE_SEPARATOR);
165163
}
166164

167165
if (withContext) {
168-
appendLoggerContext(sb, event.getLoggerContextVO());
166+
// at this stage we assume that at least one field was written
169167
sb.append(VALUE_SEPARATOR);
168+
appendLoggerContext(sb, event.getLoggerContextVO());
170169
}
170+
171171
if (withMarkers)
172172
appendMarkers(sb, event);
173173

@@ -178,17 +178,18 @@ public byte[] encode(ILoggingEvent event) {
178178
appendKeyValuePairs(sb, event);
179179

180180
if (withMessage) {
181-
appenderMember(sb, MESSAGE_ATTR_NAME, jsonEscape(event.getMessage()));
182181
sb.append(VALUE_SEPARATOR);
182+
appenderMember(sb, MESSAGE_ATTR_NAME, jsonEscape(event.getMessage()));
183183
}
184184

185185
if (withFormattedMessage) {
186-
appenderMember(sb, FORMATTED_MESSAGE_ATTR_NAME, jsonEscape(event.getFormattedMessage()));
187186
sb.append(VALUE_SEPARATOR);
187+
appenderMember(sb, FORMATTED_MESSAGE_ATTR_NAME, jsonEscape(event.getFormattedMessage()));
188188
}
189189

190-
if (withArguments)
190+
if (withArguments) {
191191
appendArgumentArray(sb, event);
192+
}
192193

193194
if (withThrowable)
194195
appendThrowableProxy(sb, THROWABLE_ATTR_NAME, event.getThrowableProxy());
@@ -198,6 +199,19 @@ public byte[] encode(ILoggingEvent event) {
198199
return sb.toString().getBytes(UTF_8_CHARSET);
199200
}
200201

202+
void appendValueSeparator(StringBuilder sb, boolean... subsequentConditionals) {
203+
boolean enabled = false;
204+
for (boolean subsequent : subsequentConditionals) {
205+
if (subsequent) {
206+
enabled = true;
207+
break;
208+
}
209+
}
210+
211+
if (enabled)
212+
sb.append(VALUE_SEPARATOR);
213+
}
214+
201215
private void appendLoggerContext(StringBuilder sb, LoggerContextVO loggerContextVO) {
202216

203217
sb.append(QUOTE).append(CONTEXT_ATTR_NAME).append(QUOTE_COL);
@@ -240,6 +254,13 @@ private void appendMap(StringBuilder sb, String attrName, Map<String, String> ma
240254
}
241255

242256
private void appendThrowableProxy(StringBuilder sb, String attributeName, IThrowableProxy itp) {
257+
appendThrowableProxy(sb, attributeName, itp, true);
258+
}
259+
260+
private void appendThrowableProxy(StringBuilder sb, String attributeName, IThrowableProxy itp, boolean appendValueSeparator) {
261+
262+
if (appendValueSeparator)
263+
sb.append(VALUE_SEPARATOR);
243264

244265
// in the nominal case, attributeName != null. However, attributeName will be null for suppressed
245266
// IThrowableProxy array, in which case no attribute name is needed
@@ -273,7 +294,6 @@ private void appendThrowableProxy(StringBuilder sb, String attributeName, IThrow
273294

274295
IThrowableProxy cause = itp.getCause();
275296
if (cause != null) {
276-
sb.append(VALUE_SEPARATOR);
277297
appendThrowableProxy(sb, CAUSE_ATTR_NAME, cause);
278298
}
279299

@@ -282,14 +302,12 @@ private void appendThrowableProxy(StringBuilder sb, String attributeName, IThrow
282302
sb.append(VALUE_SEPARATOR);
283303
sb.append(QUOTE).append(SUPPRESSED_ATTR_NAME).append(QUOTE_COL);
284304
sb.append(OPEN_ARRAY);
305+
285306
boolean first = true;
286307
for (IThrowableProxy suppressedITP : suppressedArray) {
287-
if (first) {
308+
appendThrowableProxy(sb, null, suppressedITP, !first);
309+
if (first)
288310
first = false;
289-
} else {
290-
sb.append(VALUE_SEPARATOR);
291-
}
292-
appendThrowableProxy(sb, null, suppressedITP);
293311
}
294312
sb.append(CLOSE_ARRAY);
295313
}
@@ -350,6 +368,7 @@ private void appendKeyValuePairs(StringBuilder sb, ILoggingEvent event) {
350368
if (kvpList == null || kvpList.isEmpty())
351369
return;
352370

371+
sb.append(VALUE_SEPARATOR);
353372
sb.append(QUOTE).append(KEY_VALUE_PAIRS_ATTR_NAME).append(QUOTE_COL).append(SP).append(OPEN_ARRAY);
354373
final int len = kvpList.size();
355374
for (int i = 0; i < len; i++) {
@@ -361,14 +380,14 @@ private void appendKeyValuePairs(StringBuilder sb, ILoggingEvent event) {
361380
sb.append(CLOSE_OBJ);
362381
}
363382
sb.append(CLOSE_ARRAY);
364-
sb.append(VALUE_SEPARATOR);
365383
}
366384

367385
private void appendArgumentArray(StringBuilder sb, ILoggingEvent event) {
368386
Object[] argumentArray = event.getArgumentArray();
369387
if (argumentArray == null)
370388
return;
371389

390+
sb.append(VALUE_SEPARATOR);
372391
sb.append(QUOTE).append(ARGUMENT_ARRAY_ATTR_NAME).append(QUOTE_COL).append(SP).append(OPEN_ARRAY);
373392
final int len = argumentArray.length;
374393
for (int i = 0; i < len; i++) {
@@ -378,14 +397,14 @@ private void appendArgumentArray(StringBuilder sb, ILoggingEvent event) {
378397

379398
}
380399
sb.append(CLOSE_ARRAY);
381-
sb.append(VALUE_SEPARATOR);
382400
}
383401

384402
private void appendMarkers(StringBuilder sb, ILoggingEvent event) {
385403
List<Marker> markerList = event.getMarkerList();
386404
if (markerList == null)
387405
return;
388406

407+
sb.append(VALUE_SEPARATOR);
389408
sb.append(QUOTE).append(MARKERS_ATTR_NAME).append(QUOTE_COL).append(SP).append(OPEN_ARRAY);
390409
final int len = markerList.size();
391410
for (int i = 0; i < len; i++) {
@@ -395,8 +414,6 @@ private void appendMarkers(StringBuilder sb, ILoggingEvent event) {
395414

396415
}
397416
sb.append(CLOSE_ARRAY);
398-
sb.append(VALUE_SEPARATOR);
399-
400417
}
401418

402419
private String jsonEscapedToString(Object o) {
@@ -419,7 +436,7 @@ private String jsonEscape(String s) {
419436

420437
private void appendMDC(StringBuilder sb, ILoggingEvent event) {
421438
Map<String, String> map = event.getMDCPropertyMap();
422-
439+
sb.append(VALUE_SEPARATOR);
423440
sb.append(QUOTE).append(MDC_ATTR_NAME).append(QUOTE_COL).append(SP).append(OPEN_OBJ);
424441
if (isNotEmptyMap(map)) {
425442
Set<Map.Entry<String, String>> entrySet = map.entrySet();
@@ -433,8 +450,6 @@ private void appendMDC(StringBuilder sb, ILoggingEvent event) {
433450

434451
}
435452
sb.append(CLOSE_OBJ);
436-
sb.append(VALUE_SEPARATOR);
437-
438453
}
439454

440455
boolean isNotEmptyMap(Map map) {

‎logback-classic/src/test/java/ch/qos/logback/classic/encoder/JsonEncoderTest.java

+18-3
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,8 @@ void smoke() throws JsonProcessingException {
9999
byte[] resultBytes = jsonEncoder.encode(event);
100100
String resultString = new String(resultBytes, StandardCharsets.UTF_8);
101101
//System.out.println(resultString);
102-
103102
JsonLoggingEvent resultEvent = stringToLoggingEventMapper.mapStringToLoggingEvent(resultString);
104103
compareEvents(event, resultEvent);
105-
106104
}
107105

108106
@Test
@@ -219,7 +217,7 @@ void withFormattedMessage() throws JsonProcessingException {
219217

220218
byte[] resultBytes = jsonEncoder.encode(event);
221219
String resultString = new String(resultBytes, StandardCharsets.UTF_8);
222-
System.out.println(resultString);
220+
//System.out.println(resultString);
223221

224222
JsonLoggingEvent resultEvent = stringToLoggingEventMapper.mapStringToLoggingEvent(resultString);
225223
compareEvents(event, resultEvent);
@@ -257,6 +255,23 @@ void withThrowable() throws JsonProcessingException {
257255
compareEvents(event, resultEvent);
258256
}
259257

258+
@Test
259+
void withThrowableDisabled() throws JsonProcessingException {
260+
Throwable t = new RuntimeException("withThrowableDisabled");
261+
LoggingEvent event = new LoggingEvent("in withThrowable test", logger, Level.WARN, "hello kvp", t, null);
262+
jsonEncoder.setWithThrowable(false);
263+
byte[] resultBytes = jsonEncoder.encode(event);
264+
String resultString = new String(resultBytes, StandardCharsets.UTF_8);
265+
//System.out.println(resultString);
266+
JsonLoggingEvent resultEvent = stringToLoggingEventMapper.mapStringToLoggingEvent(resultString);
267+
268+
LoggingEvent eventWithNoThrowable = new LoggingEvent("in withThrowable test", logger, Level.WARN, "hello kvp", null, null);
269+
eventWithNoThrowable.setTimeStamp(event.getTimeStamp());
270+
271+
compareEvents(eventWithNoThrowable, resultEvent);
272+
}
273+
274+
260275
@Test
261276
void withThrowableHavingCause() throws JsonProcessingException {
262277
Throwable cause = new IllegalStateException("test cause");

0 commit comments

Comments
 (0)