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

Record session statistics on pn.state.session_info #1615

Merged
merged 10 commits into from
Oct 8, 2020

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Oct 7, 2020

Stores various information about user sessions on pn.state.session_info. Currently this records the following fields per session:

  • started (float): Timestamp when the session was started

  • rendered (float): Timestamp when the session was fully rendered

  • ended (float): Timestamp when the session was closed on the server

  • user_agent (str): A string identifying the user agent

  • Add tests

  • Add docs

@jbednar
Copy link
Member

jbednar commented Oct 7, 2020

Cool! Will the session info ever expire? If not, won't storing that data lead to ever-increasing memory usage over time?

@philippjfr philippjfr force-pushed the record_session_info branch from d283d75 to 11c8e3c Compare October 7, 2020 18:00
@philippjfr
Copy link
Member Author

Cool! Will the session info ever expire? If not, won't storing that data lead to ever-increasing memory usage over time?

This is true, that said it's about 1.6 KB per session so even at 100k sessions that's not a huge amount. What I might suggest is that it keeps only the most recent N sessions where N is configurable.

@philippjfr
Copy link
Member Author

Okay, I've now reduced it to 0.6 kB per session and added the config.session_history option.

@codecov
Copy link

codecov bot commented Oct 8, 2020

Codecov Report

Merging #1615 into master will decrease coverage by 0.14%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1615      +/-   ##
==========================================
- Coverage   85.40%   85.25%   -0.15%     
==========================================
  Files         147      147              
  Lines       16610    16660      +50     
==========================================
+ Hits        14185    14204      +19     
- Misses       2425     2456      +31     
Impacted Files Coverage Δ
panel/command/serve.py 18.08% <0.00%> (-1.02%) ⬇️
panel/io/rest.py 26.04% <20.00%> (-0.93%) ⬇️
panel/io/state.py 45.50% <40.00%> (-0.33%) ⬇️
panel/io/server.py 65.02% <45.00%> (-1.80%) ⬇️
panel/viewable.py 70.38% <70.00%> (-0.54%) ⬇️
panel/config.py 46.39% <100.00%> (+0.18%) ⬆️
panel/io/__init__.py 100.00% <100.00%> (ø)
panel/pane/base.py 89.50% <100.00%> (-0.06%) ⬇️
panel/param.py 89.78% <100.00%> (-0.03%) ⬇️
panel/tests/pane/test_markup.py 99.23% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23184e3...48bd49e. Read the comment docs.

@jbednar
Copy link
Member

jbednar commented Oct 8, 2020

What I might suggest is that it keeps only the most recent N sessions where N is configurable.

Sounds good. It should probably also keep the number of sessions as a global value, so that we can tell how much data has been lost due to reaching the session_history limit.

panel/config.py Outdated Show resolved Hide resolved
@philippjfr philippjfr force-pushed the record_session_info branch from 511a8d5 to 48bd49e Compare October 8, 2020 13:06
@philippjfr philippjfr merged commit 83cfe80 into master Oct 8, 2020
@philippjfr philippjfr deleted the record_session_info branch October 8, 2020 13:22
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.

2 participants