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.

Sign up for GitHub

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 6 commits into
base: main
Choose a base branch
from
Open

Allow using precise floats in logs #2005

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
712af2a
Allow using precise floats in logs
kasmarian Jan 10, 2025
27515ed
extract usePreciseFloats check into a separate method
kasmarian Jan 13, 2025
65bcb76
add a comment about performance penalty
kasmarian Jan 13, 2025
20ba266
add strategy to handle different ways of handling floats
kasmarian Jan 17, 2025
2499c37
leve only one level of wrappers (remove creators)
kasmarian Jan 20, 2025
4620501
update README
kasmarian Jan 20, 2025
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
14 changes: 14 additions & 0 deletions README.md
View file
Open in desktop
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
4 changes: 4 additions & 0 deletions logbook-json/src/main/java/org/zalando/logbook/json/CompactingJsonBodyFilter.java
View file
Open in desktop
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
6 changes: 6 additions & 0 deletions logbook-json/src/main/java/org/zalando/logbook/json/DefaultJsonGeneratorWrapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.zalando.logbook.json;


final class DefaultJsonGeneratorWrapper implements JsonGeneratorWrapper {

}
14 changes: 11 additions & 3 deletions logbook-json/src/main/java/org/zalando/logbook/json/JacksonJsonFieldBodyFilter.java
View file
Open in desktop
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
14 changes: 14 additions & 0 deletions logbook-json/src/main/java/org/zalando/logbook/json/JsonGeneratorWrapper.java
View file
Open in desktop
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);
}

}
18 changes: 18 additions & 0 deletions logbook-json/src/main/java/org/zalando/logbook/json/NumberAsStringJsonGeneratorWrapper.java
View file
Open in desktop
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);
}
}
}
16 changes: 14 additions & 2 deletions logbook-json/src/main/java/org/zalando/logbook/json/ParsingJsonCompactor.java
View file
Open in desktop
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
14 changes: 14 additions & 0 deletions logbook-json/src/main/java/org/zalando/logbook/json/PreciseFloatJsonGeneratorWrapper.java
View file
Open in desktop
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);
}
}
11 changes: 9 additions & 2 deletions logbook-json/src/main/java/org/zalando/logbook/json/PrettyPrintingJsonBodyFilter.java
View file
Open in desktop
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
21 changes: 19 additions & 2 deletions logbook-json/src/test/java/org/zalando/logbook/json/CompactingJsonBodyFilterTest.java
View file
Open in desktop
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\"}}";
Copy link
Member Author

Choose a reason for hiding this comment

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

@msdousti as per WRITE_NUMBERS_AS_STRINGS feature, it looks like it doesn't work the way we thought.

JsonToken type is still ID_NUMBER_FLOAT and JsonGenerator calls _copyCurrentFloatValue, which calls
WriterBasedJsonGenerator.writeNumber(double d) and the double value is written down to the write, just wrapped in a string.

I left this failing test as well as NumberAsStringJsonGeneratorWrapperCreator implementation as a showcase.

Copy link
Collaborator

@msdousti msdousti Jan 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@kasmarian

I should admit enable(JsonWriteFeature.WRITE_NUMBERS_AS_STRINGS) was a bad suggestion as it is used during writing of an object to JSON, not reading of a JSON string.

Unfortunately, I could not find a similar JsonReadFeature.

Alternatively, I came up with the following code. The compact function can be adapted to look like this or something similar.

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

import java.io.CharArrayWriter;

import static com.fasterxml.jackson.core.JsonToken.VALUE_NUMBER_FLOAT;

enum JsonNumberFormat {
    AS_FLOAT,
    AS_DOUBLE,
    AS_BIG_DECIMAL,
    AS_STRING,
    AS_DEFAULT,
}

public class Example {


    private static String readJson(String json, JsonNumberFormat format) throws Exception {
        final JsonFactory factory = new JsonFactory();

        try (
                final CharArrayWriter output = new CharArrayWriter(json.length());
                final JsonParser parser = factory.createParser(json);
                final JsonGenerator generator = factory.createGenerator(output)) {

            while (parser.nextToken() != null) {
                if (parser.getCurrentToken() == VALUE_NUMBER_FLOAT) {
                    switch (format) {
                        case AS_FLOAT -> generator.writeNumber(parser.getFloatValue());
                        case AS_DOUBLE -> generator.writeNumber(parser.getValueAsDouble());
                        case AS_BIG_DECIMAL -> generator.writeNumber(parser.getDecimalValue());
                        case AS_STRING -> generator.writeString(parser.getValueAsString());
                        default -> generator.copyCurrentEvent(parser);
                    }
                } else {
                    generator.copyCurrentEvent(parser);
                }
            }

            generator.flush();
            return output.toString();
        }
    }

    public static void main(String[] args) throws Exception {
        final String json = """
            {
                "x": [0.40000000000000002, 1, "a"]
            }
            """;

        for (JsonNumberFormat format : JsonNumberFormat.values()) {
            System.out.println(format + " --> " + readJson(json, format));
        }
    }
}

Output:

AS_FLOAT --> {"x":[0.4,1,"a"]}
AS_DOUBLE --> {"x":[0.4,1,"a"]}
AS_BIG_DECIMAL --> {"x":[0.40000000000000002,1,"a"]}
AS_STRING --> {"x":["0.40000000000000002",1,"a"]}
AS_DEFAULT --> {"x":[0.4,1,"a"]}

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion with using bare (parser.getValueAsString())! It's straightforward, and the functionality is on the same level, that these filters are operating on, as opposed to factory feature flags, and I didn't like as much. I applied it in NumberAsStringJsonGeneratorWrapper. Please have a look.
If you think we should switch from the strategy approach to feature flags (enums), let's discuss it in the other thread, please.

assertThat(filtered).isEqualTo(compactedWithFloatAsString);
}

@Test
void shouldSkipInvalidJsonLookingLikeAValidOne() {
final String invalidJson = "{invalid}";
Expand Down
19 changes: 19 additions & 0 deletions logbook-json/src/test/java/org/zalando/logbook/json/JacksonJsonFieldBodyFilterTest.java
View file
Open in desktop
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
5 changes: 3 additions & 2 deletions logbook-json/src/test/java/org/zalando/logbook/json/JsonHttpLogFormatterTest.java
View file
Open in desktop
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
22 changes: 20 additions & 2 deletions logbook-json/src/test/java/org/zalando/logbook/json/PrettyPrintingJsonBodyFilterTest.java
View file
Open in desktop
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
View file
Open in desktop
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
}
Loading