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

Disk Buffer Functionality Not Available In Sentry Java 3? #1056

Closed
mbitar-gm opened this issue Nov 20, 2020 · 11 comments
Closed

Disk Buffer Functionality Not Available In Sentry Java 3? #1056

mbitar-gm opened this issue Nov 20, 2020 · 11 comments
Labels
enhancement New feature or request

Comments

@mbitar-gm
Copy link

mbitar-gm commented Nov 20, 2020

The sentry disk buffer functionality for offline reporting doesn't seem to be present in the 3.* SDK. And there is nothing about it in the docs.
I am using it with a log4j2 integration.

Is is actually available? If yes, how to enable it in sdk 3?
If not, is there any ways to handle offline reporting?

If this is no longer supported what's the strategy for handling lack of connectivity?

@marandaneto marandaneto added the enhancement New feature or request label Nov 21, 2020
@marandaneto
Copy link
Contributor

marandaneto commented Nov 21, 2020

@michelbitar-99 thanks for raising this.

It's not supported ootb because usually backend apps are online 100% of the time and also because IO could become a bottleneck if there a ton of requests per second.

We do support the disk buffer for Android and they share the same code base, so it'd not be that hard to bring this in, I'm going to discuss this internally.

cc @bruno-garcia

@tlf30
Copy link

tlf30 commented Nov 21, 2020

Having the disk buffer for java applications that my team is very interested in. This was a valuable feature for us in version 1 as many of our normal java applications and wildfly applications only touch the internet periodically.

@jadbaz
Copy link

jadbaz commented Nov 22, 2020

Thanks @michelbitar-99, I have the same concern

@marandaneto, I'd say the assumption that apps using the Java SDK are online 100% of the time usually holds except in a few very important cases when it doesn't.

  • Backend apps can be online but lose connectivity to Sentry. Sentry could be down. Some temporary firewall issues or other IT hiccups can block outgoing access to Sentry for some time..
  • In case of on-premise installations of Sentry, especially with smaller orgs, 0-downtime of Sentry itself is not always possible. We would like not to lose errors generated during Sentry maintenance windows.
  • Finally, and I'd say most importantly, the Java SDK can also be used for non-backend apps. Our org uses it in an IOT-like setup where connectivity is not always guaranteed. It's actually one of the top reasons we chose Sentry in the first place.

@marandaneto
Copy link
Contributor

@jadbaz thanks for your insightful comment, we'll for sure consider it, but I can't promise any deadlines, for now, focusing on the Performance feature.

As I said, it's not supported ootb but the code base has all the necessary bits, if you create your own SentryOptions and Hub, setting all the necessary bits, you can achieve this easily, no code changes required.
You basically need to mimic the Android bits which live here:

https://github.com/getsentry/sentry-java/blob/main/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java#L211

https://github.com/getsentry/sentry-java/blob/main/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java#L118-L120

Ideally, we'd expose a bool to toggle this feature automatically.
Also, cached events would be sent only on App. restart which is suboptimal for Server apps, a mechanism like #912 would improve this.

If one of you feels submitting a PR, I'd love to guide & review btw.

@jadbaz
Copy link

jadbaz commented Nov 23, 2020

Thank you @marandaneto, I guess for now we'll stick to the legacy SDK and we'll impatiently wait for this feature to be added to the new SDK.

@jadbaz
Copy link

jadbaz commented Jan 20, 2021

@marandaneto
I know it's only been 2 months but we're about to get started with this and it would be really helpful to know the general direction
Whether or not this is on the roadmap
Thank you

@marandaneto
Copy link
Contributor

@bruno-garcia for visibility

@bruno-garcia
Copy link
Member

Sounds like all you need to do is to set options.setCacheDirPath to some path and the SDK does its magic. Worth noting this on a server isn't something we've tested since it was built for Android.

Also worth noting this will not work with the apache transport released on version 4.0.0.

Let us know if you've tried it out. 👍

@jadbaz
Copy link

jadbaz commented Mar 5, 2021

@bruno-garcia, thanks
But I still don't really understand how this solves the problem

setCacheDirPath will have Sentry buffer events to disk.
Those events will be resent once the application is restarted.
They will ONLY be sent if the application is restarted
This works in the context of an android app that would get restarted every so often but not if the application is always up

The legacy SDK buffering functionality used to resend cached events on an interval (buffer.flushtime) while the application is still up

@marandaneto
Copy link
Contributor

@jadbaz you are right, it'd work for Desktop Apps but not Server Apps

@bruno-garcia
Copy link
Member

bruno-garcia commented Mar 6, 2021

I'll close this issue since the feature is available.
For sending periodically we'll track on #912
For better documenting this: #1309

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants