Some corrections and performance improvements in CorrelationIdFilter#183
Conversation
…tring array and two not necessary substrings. * Remove not(<>) predicate since it leads (very likely) to allocation. By using ?: it could be formulated without negation. * traceparent parsing without String[] and not required substring creation. * Fix debug log entry. Additional parameter was given but without corresponding format specifier.
KarstenSchnitter
left a comment
KarstenSchnitter
left a comment
There was a problem hiding this comment.
Thanks, for providing this PR. I will run the jmh tests to see, what benefit this implementation brings. Please clarify the removal of the custom field in the log message.
| LOG.debug("Generated new correlation-id <{}>", correlationId, customField(correlationHeader.getField(), | ||
| correlationId)); | ||
| LOG.debug("Generated new correlation-id <{}>", correlationId); |
There was a problem hiding this comment.
Why do you think, the custom field can go? By what mechanism will it be part of this log message?
There was a problem hiding this comment.
I removed it because, there is no corresponding {} to the field in the format string.
There are two solutions to this, either enhancing the format string and keep the field entry (I assume that's your intend) or as I did - remove the currently not used argument.
There was a problem hiding this comment.
Actually, this is a silent parameter. It is not rendered in the pattern but still picked-up by the JSONEncoder. This will render a custom field "correlation-id": "UUID" in the log message. It is the work-aroung in SLF4J 1.x, where there are no key-value-pairs available for this purpose.
There was a problem hiding this comment.
Thanks for the explanation, I'll revert this part.
...support-servlet/src/main/java/com/sap/hcp/cf/logging/servlet/filter/CorrelationIdFilter.java
Show resolved
Hide resolved
2 participants
Uh oh!
There was an error while loading. Please reload this page.