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 4 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
12 changes: 12 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,18 @@ 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. Using this flag
> may lead to a performance penalty as BigDecimal is usually used as the representation accessed from JsonParser.
>
> 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
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 JsonGeneratorWrapperCreator jsonGeneratorWrapperCreator) {
this(new ParsingJsonCompactor(jsonGeneratorWrapperCreator));
}

public CompactingJsonBodyFilter() {
this(new ParsingJsonCompactor());
}
Expand Down
9 changes: 9 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,9 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonGenerator;

final class DefaultJsonGeneratorWrapper extends JsonGeneratorWrapper {
public DefaultJsonGeneratorWrapper(JsonGenerator delegate) {
super(delegate);
}
}
12 changes: 12 additions & 0 deletions logbook-json/src/main/java/org/zalando/logbook/json/DefaultJsonGeneratorWrapperCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;

import java.io.CharArrayWriter;
import java.io.IOException;

public final class DefaultJsonGeneratorWrapperCreator implements JsonGeneratorWrapperCreator {
public JsonGeneratorWrapper create(JsonFactory factory, CharArrayWriter output) throws IOException {
return new DefaultJsonGeneratorWrapper(factory.createGenerator(output));
}
}
15 changes: 11 additions & 4 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
@@ -1,7 +1,6 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -29,11 +28,20 @@ public class JacksonJsonFieldBodyFilter implements BodyFilter {
private final String replacement;
private final Set<String> fields;
private final JsonFactory factory;
private final JsonGeneratorWrapperCreator jsonGeneratorWrapperCreator;

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

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

public JacksonJsonFieldBodyFilter(final Collection<String> fieldNames, final String replacement) {
Expand All @@ -49,11 +57,10 @@ public String filter(final String body) {
try ( final CharArrayWriter writer = new CharArrayWriter(body.length() * 2) ){ // rough estimate of final size)

try (final JsonParser parser = factory.createParser(body);
final JsonGenerator generator = factory.createGenerator(writer)){
final JsonGeneratorWrapper generator = jsonGeneratorWrapperCreator.create(factory, writer)){

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

generator.copyCurrentEvent(parser);
if (nextToken == JsonToken.FIELD_NAME && fields.contains(parser.currentName())) {
nextToken = parser.nextToken();
Expand Down
38 changes: 38 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,38 @@
package org.zalando.logbook.json;

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

import java.io.Closeable;
import java.io.Flushable;
import java.io.IOException;

public abstract class JsonGeneratorWrapper implements Closeable, Flushable {
protected final JsonGenerator delegate;

public JsonGeneratorWrapper(JsonGenerator delegate) {
this.delegate = delegate;
}

public void copyCurrentEvent(JsonParser parser) throws IOException {
delegate.copyCurrentEvent(parser);
}

public void useDefaultPrettyPrinter() {
delegate.useDefaultPrettyPrinter();
}

@Override
public void close() throws IOException {
delegate.close();
}

@Override
public void flush() throws IOException {
delegate.flush();
}

public void writeString(String text) throws IOException {
delegate.writeString(text);
}
}
10 changes: 10 additions & 0 deletions logbook-json/src/main/java/org/zalando/logbook/json/JsonGeneratorWrapperCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;

import java.io.CharArrayWriter;
import java.io.IOException;

public interface JsonGeneratorWrapperCreator {
JsonGeneratorWrapper create(JsonFactory factory, CharArrayWriter output) throws IOException;
}
11 changes: 11 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,11 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonGenerator;

final class NumberAsStringJsonGeneratorWrapper extends JsonGeneratorWrapper {
@SuppressWarnings("deprecation")
public NumberAsStringJsonGeneratorWrapper(JsonGenerator delegate) {
super(delegate);
delegate.enable(JsonGenerator.Feature.WRITE_NUMBERS_AS_STRINGS);
}
}
19 changes: 19 additions & 0 deletions ...son/src/main/java/org/zalando/logbook/json/NumberAsStringJsonGeneratorWrapperCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.json.JsonWriteFeature;

import java.io.CharArrayWriter;
import java.io.IOException;


public final class NumberAsStringJsonGeneratorWrapperCreator implements JsonGeneratorWrapperCreator {
public JsonGeneratorWrapper create(JsonFactory factory, CharArrayWriter output) throws IOException {
final JsonGenerator generator = factory.rebuild()
.enable(JsonWriteFeature.WRITE_NUMBERS_AS_STRINGS)
.build()
.createGenerator(output);
return new NumberAsStringJsonGeneratorWrapper(generator);
}
}
16 changes: 13 additions & 3 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
@@ -1,7 +1,6 @@
package org.zalando.logbook.json;

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

import java.io.CharArrayWriter;
Expand All @@ -11,20 +10,31 @@ final class ParsingJsonCompactor implements JsonCompactor {

private final JsonFactory factory;

private final JsonGeneratorWrapperCreator jsonGeneratorWrapperCreator;

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

public ParsingJsonCompactor(final JsonGeneratorWrapperCreator jsonGeneratorWrapperCreator) {
this(new JsonFactory(), jsonGeneratorWrapperCreator);
}

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

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

@Override
public String compact(final String json) throws IOException {
try (
final CharArrayWriter output = new CharArrayWriter(json.length());
final JsonParser parser = factory.createParser(json);
final JsonGenerator generator = factory.createGenerator(output)) {
final JsonGeneratorWrapper generator = jsonGeneratorWrapperCreator.create(factory, output)) {

while (parser.nextToken() != null) {
generator.copyCurrentEvent(parser);
Expand Down
18 changes: 18 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,18 @@
package org.zalando.logbook.json;

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

import java.io.IOException;

final class PreciseFloatJsonGeneratorWrapper extends JsonGeneratorWrapper {

public PreciseFloatJsonGeneratorWrapper(JsonGenerator delegate) {
super(delegate);
}

@Override
public void copyCurrentEvent(JsonParser parser) throws IOException {
delegate.copyCurrentEventExact(parser);
}
}
12 changes: 12 additions & 0 deletions ...-json/src/main/java/org/zalando/logbook/json/PreciseFloatJsonGeneratorWrapperCreator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;

import java.io.CharArrayWriter;
import java.io.IOException;

public final class PreciseFloatJsonGeneratorWrapperCreator implements JsonGeneratorWrapperCreator {
public JsonGeneratorWrapper create(JsonFactory factory, CharArrayWriter output) throws IOException {
return new PreciseFloatJsonGeneratorWrapper(factory.createGenerator(output));
}
}
12 changes: 9 additions & 3 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
@@ -1,7 +1,6 @@
package org.zalando.logbook.json;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -20,9 +19,16 @@
public final class PrettyPrintingJsonBodyFilter implements BodyFilter {

private final JsonFactory factory;
private final JsonGeneratorWrapperCreator jsonGeneratorWrapperCreator;

public PrettyPrintingJsonBodyFilter(final JsonFactory factory) {
public PrettyPrintingJsonBodyFilter(final JsonFactory factory,
final JsonGeneratorWrapperCreator jsonGeneratorWrapperCreator) {
this.factory = factory;
this.jsonGeneratorWrapperCreator = jsonGeneratorWrapperCreator;
}

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

public PrettyPrintingJsonBodyFilter() {
Expand All @@ -47,7 +53,7 @@ public String filter(@Nullable final String contentType, final String body) {
try (
final CharArrayWriter output = new CharArrayWriter(body.length() * 2); // rough estimate of output size
final JsonParser parser = factory.createParser(body);
final JsonGenerator generator = factory.createGenerator(output)) {
final JsonGeneratorWrapper generator = jsonGeneratorWrapperCreator.create(factory, output)) {

generator.useDefaultPrettyPrinter();

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 PreciseFloatJsonGeneratorWrapperCreator())
.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 NumberAsStringJsonGeneratorWrapperCreator())
.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
10 changes: 10 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,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(), new PreciseFloatJsonGeneratorWrapperCreator());
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
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
Loading
Loading