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

Tracy without session - concept of using files as a storage [WIP] #362

Closed
wants to merge 1 commit into from
Closed

Tracy without session - concept of using files as a storage [WIP] #362

wants to merge 1 commit into from

Conversation

forrest79
Copy link
Contributor

We have a project, where session is started only for logged users. It's a little bit tricky, that Tracy in devel mode always starts session. This could possible hide some bug on devel environment, that is, in better scenario catch with tests, in worse scenario on production :-)

This is just concept, how to let developer choose between session as storage (default and works the same way as now) or file storage. In configuration, you can enable it with setting directory for data files:

tracy:
	fileStorage: %tempDir%

If this concept will be approved, I can prepare PR for actual Tracy version.

@dg
Copy link
Member

dg commented Apr 3, 2019

Thanks a lot for PR.

I need to understand why you're doing this. From my point of view:

  • sessions is a storage on a disk whose identifier is transferred in a cookie.
  • your FileStorage is a storage on a disk whose identifier is transferred in a cookie

I simply don't understand why to reimplement session by own class.

I used to do something like that a long time ago e1e204c#diff-20100846c73ff8ebc44f72ccbbf13c22, but now I don't remember why :-) and there was issue about it #154

@forrest79
Copy link
Contributor Author

forrest79 commented Apr 3, 2019

I tried to show our reasons in description, maybe not enough clear :-) Imagine web, that in some cases use PHP session (in our case for logged users and some other minor use cases...) but for most requests (~80%), you're not starting session and web needs to cover both variants - session could be running or not. And when you're on your local development machine with Tracy, PHP session is always running, so you can't simply simulate environment without session and is very easy to create a bug. With this PR, you can choose to use "Tracy session" and PHP session is working in the same way as on production web.

By default, PHP session is still used, so with this, I think, that you can cover #154. This is just for those, who don't wat to start PHP session because of Tracy.

@dg
Copy link
Member

dg commented Apr 3, 2019

Ok I get it.

@forrest79
Copy link
Contributor Author

I think, that maybe there is no need to have FileStorage in Tracy repository, just ability to use your own storage could be also suitable.

@ikeblaster
Copy link

ikeblaster commented Sep 25, 2019

@dg I have probably found another reason to implement this.

TLDR: Session handlers (e.g. memcached) can have size limits. Some tracy info easily becomes too big and this can even lead to session destruction.


It's actually quite common situation - when you have request with redirect, then the panel is saved into session. Together with memcached as a session handler it can easily lead to hitting the session size limit. In our case the result is completely deleted session (without any error message visible) and user being redirected to a login page. This was really weird at first and I wonder how nobody has ever met that before (or at least google doesn't return to me anything similar).

On production it's obviously not a problem, there is no tracy, we have this on our dev site. So far I have just switched to files session handler, but I would rather see better solution. It's desirable to have dev environment as close as possible to production site. I was also thinking about custom session handler, which would split the session between files (just $_SESSION['_tracy']['redirect']) and memcached (the rest).

Or am I overlooking something?

@forrest79
Copy link
Contributor Author

Hi, I just want to ask, if you have though about this? We're using our fork with this update several months without any problem and right now, we're finishing update to Nette 3, so we will use this also with Tracy 2.7. Thanks for some info.

@dg
Copy link
Member

dg commented May 8, 2020

I'm going to put it in version 2.8 or the next one.

@forrest79
Copy link
Contributor Author

Great to hear that, thanks 👍

@dg dg closed this in 8fd3f93 Dec 10, 2021
dg added a commit to dg/tracy that referenced this pull request Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants