-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(rtcstats): move conference start time to ljm #2544
feat(rtcstats): move conference start time to ljm #2544
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a question.
// statisticsId -> email, this is only send by jitsi-meet if enableEmailInStats option is set. | ||
// statisticsDisplayName -> nick, this is only send by jitsi-meet if enableDisplayNameInStats option is set. | ||
// localId, this is the unique id that is used to track users throughout stats. | ||
const localId = Settings?.callStatsUserName ?? ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If no unique name is provided, shouldn't we generate sonething random, instead of using the empty string and potentially mixing things up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this would be the place to do that, the callStatsUserName is saved locally and the most common use case for it is to track session originating from the same medium (browser, app, etc).
If we were to generate something unique it won't do us much good as we already have a statsSessionId which is unique per session. Having it empty would at least signal that callStatsUserName isn't properly set in some envs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds reasonable!
Jenkins please test this please. |
1 similar comment
Jenkins please test this please. |
4003891
to
6437767
Compare
No description provided.