Allow configuring SentryJsonGenerator recursion limits etc

[bretthoerner]

These should be configurable somehow.

sentry-java/sentry/src/main/java/io/sentry/marshaller/json/SentryJsonGenerator.java

Lines 27 to 31 in 027544f

	private static final int MAX_LENGTH_LIST = 10;
	private static final int MAX_SIZE_MAP = 50;
	private static final int MAX_LENGTH_STRING = 400;
	private static final int MAX_NESTING = 3;
	private static final String ELIDED = "...";

[kwladyka]

It would be great to make right sizes by automate. Data are cut, so I will have to set here the biggest possible values. There is no info about limits also. I use old ver. all the time, because of this issue. Please fix it and make it more clever. Assumption is when i push some data I really want to see this data without any cut. So I don't want limit it by nesting or whatever. Just cut it when it is not fit after max bytes. It is my problem as developer to make data smaller when I am sending to Sentry. Send only what is needed. Cut data by automate like it looks now miss the point of debug. It doesn't help in any way at that moment. It makes Sentry useless for me.

Do you have any plan to change it?

[bretthoerner]

I plan to, just haven't had time. I'll try to give it another look.

Going by byte size does make sense.

[bretthoerner]

@kwladyka Out of curiosity, which of these you most often? Nesting? String length? Or do you actually need to inspect each value in huge lists/maps?

Looking at the code, it's a shame that there's seemingly no byte count on Jackson's JsonGenerator. It seems difficult for me to be able to track exactly how many bytes have been written so far. I would wrap it, but it seems like there isn't a clean single entry point for all writes, ugh.

[kwladyka]

Hmm at that moment this issue is mainly in XML data like for examples orders, products.

This response XML has full data about order returned by third system. It is only beginning of XML for 1 order. It has nested data about products, contractor, transport, payment, etc. So 3 nests is too low, 400 size for string also.

PS If it will be the map, size 50 will be too low probably etc. Just this limitations limit too much :)

[kwladyka]

PS2 So this limitation doesn't let me push data about order to analyse what cause a bug in system. Especially bugs comes very often from third party systems. That is the case. Reproduce error later to get the same data... no :) Orders changing in time etc. Need to know data during bug occur.

[kwladyka]

It is half a year after report this issue. I guess there are no plans to do it. Why? Am I miss something?

[bretthoerner]

Why?

Just lack of time.

You could override these and make a custom build:

sentry-java/sentry/src/main/java/io/sentry/marshaller/json/SentryJsonGenerator.java

Lines 27 to 30 in 4b81015

	private static final int MAX_LENGTH_LIST = 10;
	private static final int MAX_SIZE_MAP = 50;
	private static final int MAX_LENGTH_STRING = 400;
	private static final int MAX_NESTING = 3;

Or PRs are welcome.

[kwladyka]

I wish I could do PR, but I am Clojure developer. Probably my Java solution would be average.

Should it include this possibility of settings?
https://...@sentry.io/204682?buffer.dir=./sentry-buffer&max_length_list=100&max_size_map=500....

Any suggestion how to implement solution for this issue in this Java module?

[bretthoerner]

The main issues are looking up those configuration values at startup and passing them all the way down to the SentryJsonGenerator, and then the fact that a value too high may mean your events are rejected from Sentry itself.

I've considered a hacky/experiment solution where we just did something like,

private static final int MAX_LENGTH_LIST = Lookup.lookup('experimental.list.max-length');

With a fallback to the default value, of course. But that would just look up the configuration in the SentryJsonGenerator itself which is definitely a gross hack but would allow you and others an escape hatch (that we may not maintain) for now...

[ameshkov]

You don't need a custom build to override these. Just do something like this.

    /**
     * Our own sentry client factory implementation.
     * The purpose is to configure {@link SentryJsonGenerator}.
     */
    private static class CustomAndroidSentryClientFactory extends AndroidSentryClientFactory {

        /**
         * Construct an AndroidSentryClientFactory using the specified Android Context.
         *
         * @param ctx Android Context.
         */
        CustomAndroidSentryClientFactory(Context ctx) {
            super(ctx);
        }

        @Override
        protected JsonMarshaller createJsonMarshaller(int maxMessageLength) {
            return new CustomJsonMarshaller(maxMessageLength);
        }
    }

    /**
     * A marshaller implementation that configures {@link SentryJsonGenerator}.
     */
    private static class CustomJsonMarshaller extends JsonMarshaller {

        private final JsonFactory jsonFactory = new JsonFactory();

        CustomJsonMarshaller(int maxMessageLength) {
            super(maxMessageLength);
        }

        @Override
        protected JsonGenerator createJsonGenerator(OutputStream destination) throws IOException {
            SentryJsonGenerator sentryJsonGenerator = new SentryJsonGenerator(jsonFactory.createGenerator(destination));
            sentryJsonGenerator.setMaxLengthString(MAX_STRING_LENGTH);
            return sentryJsonGenerator;
        }
    }

[kwladyka]

@ameshkov thank you, but unfortunately I will have to also do custom build to include Java custom build for https://github.com/getsentry/sentry-clj . It sounds like a lot of overhead.

That is why I wish to see solution in official repository.

[bretthoerner]

@ameshkov makes a good point. sentry-clj even uses its own factory to override the JSON generator:

https://github.com/getsentry/sentry-clj/blob/64e3cc8b5f9aef053a271d14345ec2f785a2c3bb/src/sentry_clj/internal.clj#L23-L28

Can't you call setMaxLengthString (and others) from Clojure and avoid having to deal with Java at all?

[kwladyka]

hah I love this comment https://github.com/getsentry/sentry-clj/blob/64e3cc8b5f9aef053a271d14345ec2f785a2c3bb/src/sentry_clj/internal.clj#L2+L4

So I have to get from sentry-clj.internal/factory the JsonGenerator. Use on it setMaxLengthString. How to set this value on it? I don't see function to set something on JsonGenerator in factory.

https://github.com/getsentry/sentry-java/blob/master/sentry/src/main/java/io/sentry/marshaller/json/JsonMarshaller.java#L110
What is this value vs

sentry-java/sentry/src/main/java/io/sentry/marshaller/json/SentryJsonGenerator.java

Lines 27 to 30 in f9f04c2

	private static final int MAX_LENGTH_LIST = 10;
	private static final int MAX_SIZE_MAP = 50;
	private static final int MAX_LENGTH_STRING = 400;
	private static final int MAX_NESTING = 3;

?

What is true max for Sentry?

[bretthoerner]

It's one level deeper, so you'll need another proxy:

sentry-java/sentry/src/main/java/io/sentry/marshaller/json/JsonMarshaller.java

Line 183 in 4b81015

return new SentryJsonGenerator(jsonFactory.createGenerator(destination));

sentry-java/sentry/src/main/java/io/sentry/marshaller/json/SentryJsonGenerator.java

Lines 202 to 217 in 4b81015

	public void setMaxLengthList(int maxLengthList) {
	this.maxLengthList = maxLengthList;
	}
	public void setMaxLengthString(int maxLengthString) {
	this.maxLengthString = maxLengthString;
	}
	public void setMaxSizeMap(int maxSizeMap) {
	this.maxSizeMap = maxSizeMap;
	}
	public void setMaxNesting(int maxNesting) {
	this.maxNesting = maxNesting;
	}

Sentry has no maximum at this level or I would have just changed the values. The problem is Sentry will reject at 100k gzipped JSON and I found no clean/easy way to track the size of the generated object in Jackson. It's very customer dependent how those numbers will effect the event body.

[lobsterkatie]

Reviving this thread as the question of max depth has come up again with respect to the RN SDK. In that case, the creation of the Android client isn't exposed, so @ameshkov's solution unfortunately won't work.

Can we make maxNesting (etc) configurable the same way maxMessageLength is here? Then work can be done on the RN SDK side to allow its config to tap into these options.

[yilinjuang]

Any news?

[kwladyka]

I am afraid nothing changed.

[lobsterkatie]

If you're landing here because of the Java SDK itself, it's true, there's been no change. But if you landed here by way of getsentry/sentry-react-native#713, then you should be good to go, as the RN SDK will no longer rely on this one.

[jokefaker]

@bretthoerner Hi, I have a question. Will the sentry server side cut the text? I changed maxLengthString and maxMessageLength to a big number，I print the gzipped json string to a file, its just 17kb, and the JSON string is not cut . But when I open sentry , it is still cut.

[maciejwalkowiak]

The new Java SDK v3 uses GSON under the hood for JSON serialization making this ticket not valid anymore as we rely on GSON defaults.