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

Change SessionId to uuid-like format #4476

Merged
merged 2 commits into from
Mar 14, 2023
Merged

Conversation

hjoaquim
Copy link
Contributor

^

Benefit of having the SessionId as a UUID instead of the UNIX timestamp: avoid having users with the same UserId/AppId with the same SessionId - this would happen in the case of the user launching two terminal instances exactly at the same time.

Side effect: log files names change from gst_1678795847.2023-03-14_12.log to gst_5b698cdc-4d4d-4b2f-983a-2660cba0ea97-1678795471.2023-03-14_12.log - this comes with no other consequences, since for the logging etl we're filtering those files using LastModified S3 param.

Decided to add a time based component in the get_session_id function to double ensure uniqueness.

@hjoaquim hjoaquim added the feat XS Extra small feature label Mar 14, 2023
@hjoaquim hjoaquim requested review from Chavithra and jmaslek March 14, 2023 12:14
@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

❗ No coverage uploaded for pull request base (develop@5a69fba). Click here to learn what that means.
Patch has no changes to coverable lines.

❗ Current head 2dd4d64 differs from pull request most recent head 31dd15a. Consider uploading reports for the commit 31dd15a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             develop    #4476   +/-   ##
==========================================
  Coverage           ?   54.42%           
==========================================
  Files              ?      596           
  Lines              ?    53664           
  Branches           ?        0           
==========================================
  Hits               ?    29206           
  Misses             ?    24458           
  Partials           ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hjoaquim hjoaquim added this pull request to the merge queue Mar 14, 2023
Merged via the queue into develop with commit 1cdee7d Mar 14, 2023
@piiq piiq deleted the feature/session-id-uuid branch April 24, 2023 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat XS Extra small feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants