Skip to content

Commit

Permalink
Allow using precise floats in logs (#2005)
Browse files Browse the repository at this point in the history
* Allow using precise floats in logs

* extract usePreciseFloats check into a separate method

* add a comment about performance penalty

* add strategy to handle different ways of handling floats

...because no one likes boolean flags anymore ¯\_(ツ)_/¯

* leve only one level of wrappers (remove creators)

* update README
  • Loading branch information
kasmarian authored Jan 28, 2025
1 parent efe904b commit 1392997
Show file tree
Hide file tree
Showing 14 changed files with 169 additions and 15 deletions.
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,20 @@ 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 passing different `JsonGeneratorWrapper` implementations to the filter respective filters.
> Available wrappers:
> * `DefaultJsonGeneratorWrapper` - default implementation, which doesn't alter Jackson's `JsonGenerator` behavior
> * `NumberAsStringJsonGeneratorWrapper` - writes floating point numbers as strings, and preserves their precision.
> * `PreciseFloatJsonGeneratorWrapper` - writes floating point with precision, may lead to a performance penalty as
> BigDecimal is usually used as the representation accessed from JsonParser.

##### 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 JsonGeneratorWrapper jsonGeneratorWrapper) {
this(new ParsingJsonCompactor(jsonGeneratorWrapper));
}

public CompactingJsonBodyFilter() {
this(new ParsingJsonCompactor());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.zalando.logbook.json;


final class DefaultJsonGeneratorWrapper implements JsonGeneratorWrapper {

}
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 JsonGeneratorWrapper jsonGeneratorWrapper;

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 JsonGeneratorWrapper jsonGeneratorWrapper) {
this.fields = new HashSet<>(fieldNames); // thread safe for reading
this.replacement = replacement;
this.factory = factory;
this.jsonGeneratorWrapper = jsonGeneratorWrapper;
}

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

public JacksonJsonFieldBodyFilter(final Collection<String> fieldNames, final String replacement) {
Expand All @@ -53,8 +62,7 @@ public String filter(final String body) {

JsonToken nextToken;
while ((nextToken = parser.nextToken()) != null) {

generator.copyCurrentEvent(parser);
jsonGeneratorWrapper.copyCurrentEvent(generator, 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
@@ -0,0 +1,14 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;

import java.io.IOException;

public interface JsonGeneratorWrapper {

default void copyCurrentEvent(final JsonGenerator delegate, final JsonParser parser) throws IOException {
delegate.copyCurrentEvent(parser);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;

import java.io.IOException;

final class NumberAsStringJsonGeneratorWrapper implements JsonGeneratorWrapper {

public void copyCurrentEvent(JsonGenerator delegate, JsonParser parser) throws IOException {
if (parser.getCurrentToken() == JsonToken.VALUE_NUMBER_FLOAT) {
delegate.writeString(parser.getValueAsString());
} else {
delegate.copyCurrentEvent(parser);
}
}
}
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 JsonGeneratorWrapper jsonGeneratorWrapper;

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

public ParsingJsonCompactor(final JsonGeneratorWrapper jsonGeneratorWrapper) {
this(new JsonFactory(), jsonGeneratorWrapper);
}

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

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

@Override
Expand All @@ -26,8 +37,9 @@ public String compact(final String json) throws IOException {
final JsonParser parser = factory.createParser(json);
final JsonGenerator generator = factory.createGenerator(output)) {


while (parser.nextToken() != null) {
generator.copyCurrentEvent(parser);
jsonGeneratorWrapper.copyCurrentEvent(generator, parser);
}

generator.flush();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;

import java.io.IOException;

final class PreciseFloatJsonGeneratorWrapper implements JsonGeneratorWrapper {

@Override
public void copyCurrentEvent(JsonGenerator delegate, JsonParser parser) throws IOException {
delegate.copyCurrentEventExact(parser);
}
}
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 JsonGeneratorWrapper jsonGeneratorWrapper;

public PrettyPrintingJsonBodyFilter(final JsonFactory factory) {
public PrettyPrintingJsonBodyFilter(final JsonFactory factory,
final JsonGeneratorWrapper jsonGeneratorWrapper) {
this.factory = factory;
this.jsonGeneratorWrapper = jsonGeneratorWrapper;
}

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

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

while (parser.nextToken() != null) {
generator.copyCurrentEvent(parser);
jsonGeneratorWrapper.copyCurrentEvent(generator, parser);
}

generator.flush();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ 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}}";

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

@Test
void shouldPreserveBigFloatOnCopy() {
final String filtered = new CompactingJsonBodyFilter(new PreciseFloatJsonGeneratorWrapper())
.filter("application/custom+json", pretty);
final String compactedWithPreciseFloat = "{\"root\":{\"child\":\"text\",\"float_child\":0.40000000000000002}}";
assertThat(filtered).isEqualTo(compactedWithPreciseFloat);
}

@Test
void shouldLogFloatAsString() {
final String filtered = new CompactingJsonBodyFilter(new NumberAsStringJsonGeneratorWrapper())
.filter("application/custom+json", pretty);
final String compactedWithFloatAsString = "{\"root\":{\"child\":\"text\",\"float_child\":\"0.40000000000000002\"}}";
assertThat(filtered).isEqualTo(compactedWithFloatAsString);
}

@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,23 @@ 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(), new PreciseFloatJsonGeneratorWrapper());
final String filtered = filter.filter("application/json", string);
assertThat(filtered).contains("\"debt\":123450.40000000000000002");
}

@Test
public void shouldLogFloatAsStringOnCopy() throws Exception {
final String string = getResource("/student.json").trim();
final JacksonJsonFieldBodyFilter filter = new JacksonJsonFieldBodyFilter(Collections.singleton("balance"), "XXX", new JsonFactory(), new NumberAsStringJsonGeneratorWrapper());
final String filtered = filter.filter("application/json", string);
assertThat(filtered).contains("\"balance\":\"XXX\"");
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(), new PreciseFloatJsonGeneratorWrapper())
.filter("application/json", compacted);
assertThat(filtered).isEqualTo(compactedWithPreciseFloat);
}

}
6 changes: 4 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,7 @@
"Science": 1.9,
"PE": 4.0
},
"nickname": null
}
"nickname": null,
"debt": 123450.40000000000000002,
"balance": 0.40000000000000002
}

0 comments on commit 1392997

Please sign in to comment.