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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Auto shutdown in collector-exporter can hurt performance in some browsers #2205
Comments
CC @davidwitten and @dyladan |
Agree we shouldn't "shutdown" on unload, but we should perform a synchronous flush to ensure all telemetry is sent -- unless we have some sort of backing store (disk for node, and LocalStorage / IndexedDb (Slow!!!)) to ensure we can replay any unsent events on reload etc |
Is there any way I can flush synchronously? |
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days. |
Is this still valid after #2243 is landed? |
I think we are still using the unload event right? So the performance issue would still be there. |
No sorry |
Hi - Any update on this? This bug hurting us is causing low score of our website by PageSpeed Insights. |
It's still valid. #2243 was about |
@MSNev how would you propose to perform a synchronous flush? |
For browsers runtimes, there is not a lot of choice to ensure that we can get traces of the box, general options are
I don't know, if node (http) suffers from these same issues -- I would assume not. Resiliency, by using LocalStorage / SessionStorage is really the only option (as IndexedDb is an asynchronous API). But that too can cause storage issues / limits for the domain. eg. If the hosting application (page) also needs to store "lots" of data we can't really store too much. |
There are not really similar issues in node, but there are different ones. There is no way to make a synchronous network request in node without hacky solutions like calling a subprocess synchronously. |
Isn't sendBeacon and fetch both async? The only sync option is sync XHR which has problems like you mentioned |
I believe |
Got it. So sendBeacon is our best bet on shutdowns and if we have to use xhr we just have to hope |
just checking, can we use the So overall, to the original, we should use the |
Closing this as #4438 has landed. If this is still an issue, please reach out and I'll re-open. |
Hi 馃憢
@opentelemtry/exporter-collector
by default adds event listener for theunload
event on browser platforms.According to MDN it can hurt website's performance when doing back/forward buttons:
Also I'm not sure what's the purpose of this behaviour?
shutdown
only returns a promise of already exported items, so doing it onunload
doesn't make more sense, does it? See the code I'm referring to.Additionally,
onShutdown
is empty for node platform, so we could just remove theonInit
andonShutdown
functionalities. Am I missing something?The text was updated successfully, but these errors were encountered: