Skip to content
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

Allow using precise floats in logs #2005

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,17 @@ a JSON response body will **not** be escaped and represented as a string:
}
```


> [!NOTE]
> Logbook is using [BodyFilters](#Filtering) to inline json payload or to find fields for obfuscation.
> Filters for JSON bodies are using Jackson, which comes with a defect of dropping off precision from floating point
> numbers (see [FasterXML/jackson-core/issues/984](https://github.com/FasterXML/jackson-core/issues/984)).
>
> This can be changed by setting the `usePreciseFloats` flag to true in the filter respective filters.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe point out herr that using this setting can impose a performance penalty on the application due to encoding floats as BigDecimal.

>
> E.g. `new CompactingJsonBodyFilter(true)` will keep the precision of floating point numbers.


##### Common Log Format

The Common Log Format ([CLF](https://httpd.apache.org/docs/trunk/logs.html#common)) is a standardized text file format used by web servers when generating server log files. The format is supported via
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ public final class CompactingJsonBodyFilter implements BodyFilter {

private final JsonCompactor compactor;

public CompactingJsonBodyFilter(final boolean usePreciseFloats) {
this(new ParsingJsonCompactor(usePreciseFloats));
}

public CompactingJsonBodyFilter() {
this(new ParsingJsonCompactor());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,20 @@ public class JacksonJsonFieldBodyFilter implements BodyFilter {
private final String replacement;
private final Set<String> fields;
private final JsonFactory factory;
private final boolean usePreciseFloats;

public JacksonJsonFieldBodyFilter(final Collection<String> fieldNames, final String replacement, final JsonFactory factory) {
public JacksonJsonFieldBodyFilter(final Collection<String> fieldNames,
final String replacement,
final JsonFactory factory,
final boolean usePreciseFloats) {
this.fields = new HashSet<>(fieldNames); // thread safe for reading
this.replacement = replacement;
this.factory = factory;
this.usePreciseFloats = usePreciseFloats;
}

public JacksonJsonFieldBodyFilter(final Collection<String> fieldNames, final String replacement, final JsonFactory factory) {
this(fieldNames, replacement, factory, false);
}

public JacksonJsonFieldBodyFilter(final Collection<String> fieldNames, final String replacement) {
Expand All @@ -54,7 +63,11 @@ public String filter(final String body) {
JsonToken nextToken;
while ((nextToken = parser.nextToken()) != null) {

generator.copyCurrentEvent(parser);
if (usePreciseFloats) {
generator.copyCurrentEventExact(parser);
} else {
generator.copyCurrentEvent(parser);
}
if (nextToken == JsonToken.FIELD_NAME && fields.contains(parser.currentName())) {
nextToken = parser.nextToken();
generator.writeString(replacement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,23 @@ final class ParsingJsonCompactor implements JsonCompactor {

private final JsonFactory factory;

private final boolean usePreciseFloats;

public ParsingJsonCompactor(final JsonFactory factory, final boolean usePreciseFloats) {
this.factory = factory;
this.usePreciseFloats = usePreciseFloats;
}

public ParsingJsonCompactor(final boolean usePreciseFloats) {
this(new JsonFactory(), usePreciseFloats);
}

public ParsingJsonCompactor() {
this(new JsonFactory());
}

public ParsingJsonCompactor(final JsonFactory factory) {
this.factory = factory;
this(factory, false);
}

@Override
Expand All @@ -27,7 +38,11 @@ public String compact(final String json) throws IOException {
final JsonGenerator generator = factory.createGenerator(output)) {

while (parser.nextToken() != null) {
generator.copyCurrentEvent(parser);
if (usePreciseFloats) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this logic be refactored?

generator.copyCurrentEventExact(parser);
} else {
generator.copyCurrentEvent(parser);
}
}

generator.flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,16 @@
public final class PrettyPrintingJsonBodyFilter implements BodyFilter {

private final JsonFactory factory;
private final boolean usePreciseFloats;

public PrettyPrintingJsonBodyFilter(final JsonFactory factory) {
public PrettyPrintingJsonBodyFilter(final JsonFactory factory,
final boolean usePreciseFloats) {
this.factory = factory;
this.usePreciseFloats = usePreciseFloats;
}

public PrettyPrintingJsonBodyFilter(final JsonFactory factory) {
this(factory, false);
}

public PrettyPrintingJsonBodyFilter() {
Expand Down Expand Up @@ -52,7 +59,11 @@ public String filter(@Nullable final String contentType, final String body) {
generator.useDefaultPrettyPrinter();

while (parser.nextToken() != null) {
generator.copyCurrentEvent(parser);
if (usePreciseFloats) {
generator.copyCurrentEventExact(parser);
} else {
generator.copyCurrentEvent(parser);
}
}

generator.flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ class CompactingJsonBodyFilterTest {
/*language=JSON*/
private final String pretty = "{\n" +
" \"root\": {\n" +
" \"child\": \"text\"\n" +
" \"child\": \"text\",\n" +
" \"float_child\" : 0.40000000000000002" +
" }\n" +
"}";

/*language=JSON*/
private final String compacted = "{\"root\":{\"child\":\"text\"}}";
private final String compacted = "{\"root\":{\"child\":\"text\",\"float_child\":0.4}}";
private final String compactedWithPreciseFloat = "{\"root\":{\"child\":\"text\",\"float_child\":0.40000000000000002}}";

@Test
void shouldIgnoreEmptyBody() {
Expand Down Expand Up @@ -50,6 +52,13 @@ void shouldTransformValidJsonRequestWithCompatibleContentType() {
assertThat(filtered).isEqualTo(compacted);
}

@Test
void shouldPreserveBigFloatOnCopy() {
final String filtered = new CompactingJsonBodyFilter(true)
.filter("application/custom+json", pretty);
assertThat(filtered).isEqualTo(compactedWithPreciseFloat);
}

@Test
void shouldSkipInvalidJsonLookingLikeAValidOne() {
final String invalidJson = "{invalid}";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

Expand Down Expand Up @@ -75,6 +77,14 @@ public void doesNotFilterNonJson() throws Exception {
assertThat(filtered).contains("Ford");
}

@Test
public void shouldPreserveBigFloatOnCopy() throws Exception {
final String string = getResource("/student.json").trim();
final JacksonJsonFieldBodyFilter filter = new JacksonJsonFieldBodyFilter(Collections.emptyList(), "XXX", new JsonFactory(), true);
final String filtered = filter.filter("application/json", string);
assertThat(filtered).contains("\"debt\":123450.40000000000000002");
}

private String getResource(final String path) throws IOException {
final byte[] bytes = Files.readAllBytes(Paths.get("src/test/resources/" + path));
return new String(bytes, UTF_8);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,13 @@ void shouldNotEmbedReplacedJsonRequestBody(final HttpLogFormatter unit) throws I
void shouldEmbedCustomJsonRequestBody(final HttpLogFormatter unit) throws IOException {
final HttpRequest request = MockHttpRequest.create()
.withContentType("application/custom+json")
.withBodyAsString("{\"name\":\"Bob\"}");
.withBodyAsString("{\"name\":\"Bob\", \"float_value\": 0.40000000000000002 }");

final String json = unit.format(new SimplePrecorrelation("", systemUTC()), request);

with(json)
.assertEquals("$.body.name", "Bob");
.assertEquals("$.body.name", "Bob")
.assertEquals("$.body.float_value", 0.40000000000000002);
}

@ParameterizedTest
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Test;
import org.zalando.logbook.BodyFilter;
Expand All @@ -16,13 +17,23 @@ class PrettyPrintingJsonBodyFilterTest {
private final String pretty = Stream.of(
"{",
" \"root\" : {",
" \"child\" : \"text\"",
" \"child\" : \"text\",",
" \"float_child\" : 0.4",
" }",
"}"
).collect(Collectors.joining(System.lineSeparator()));

private final String compactedWithPreciseFloat = Stream.of(
"{",
" \"root\" : {",
" \"child\" : \"text\",",
" \"float_child\" : 0.40000000000000002",
" }",
"}"
).collect(Collectors.joining(System.lineSeparator()));

/*language=JSON*/
private final String compacted = "{\"root\":{\"child\":\"text\"}}";
private final String compacted = "{\"root\":{\"child\":\"text\", \"float_child\": 0.40000000000000002 }}";

@Test
void shouldIgnoreEmptyBody() {
Expand Down Expand Up @@ -68,4 +79,11 @@ void shouldConstructFromObjectMapper() {
assertThat(filtered).isEqualTo(pretty);
}

@Test
void shouldPreserveBigFloatOnCopy() {
final String filtered = new PrettyPrintingJsonBodyFilter(new JsonFactory(), true)
.filter("application/json", compacted);
assertThat(filtered).isEqualTo(compactedWithPreciseFloat);
}

}
5 changes: 3 additions & 2 deletions logbook-json/src/test/resources/student.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
"Science": 1.9,
"PE": 4.0
},
"nickname": null
}
"nickname": null,
"debt": 123450.40000000000000002
}
Loading