Skip to content

Comments

Add more information to PeerConnectionAnalyzer logs#14442

Merged
danxuliu merged 6 commits intomainfrom
add-more-information-to-peerconnectionanalyzer-logs
May 7, 2025
Merged

Add more information to PeerConnectionAnalyzer logs#14442
danxuliu merged 6 commits intomainfrom
add-more-information-to-peerconnectionanalyzer-logs

Conversation

@danxuliu
Copy link
Member

@danxuliu danxuliu commented Feb 17, 2025

Follow up to #14095

Requires #14419 (as otherwise there would be conflicts once that one is merged)

The additional information in the logs should help to debug connection warnings and find out if they were legit or false positives that still need to be fixed.

Although it would not be strictly needed to backport this to 29 and 30 it might help to avoid conflicts with backports of future fixes.

TODO

  • Log RTC stats also for receiver connections (the PeerConnectionAnalyer is not currently enabled for receiver connections, but it will be to detect stalled receiver connections and restart them)

@danxuliu
Copy link
Member Author

/backport to stable31

@danxuliu
Copy link
Member Author

/backport to stable30

@danxuliu
Copy link
Member Author

/backport to stable29

@danxuliu danxuliu force-pushed the add-more-information-to-peerconnectionanalyzer-logs branch from 9202347 to 9e2ccae Compare March 12, 2025 18:16
@danxuliu danxuliu marked this pull request as ready for review March 12, 2025 18:21
@danxuliu danxuliu requested a review from Antreesy March 12, 2025 18:21
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, looks good and verbose =)
Left some comments and nitpicks

danxuliu added 2 commits May 7, 2025 13:25
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the add-more-information-to-peerconnectionanalyzer-logs branch from 9e2ccae to 3c593fb Compare May 7, 2025 11:50
@danxuliu danxuliu requested a review from Antreesy May 7, 2025 12:58
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now!

danxuliu added 4 commits May 7, 2025 21:08
Otherwise it was not possible to know if the logged stats belonged to
the video or screen peer.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The raw RTC stats provide additional information that is sometimes
needed to debug the connection quality warning, so they are now included
in the logs.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Formatted strings were used instead of just a list of strings to ensure
the proper format (as browsers typically add a space between arguments,
but the Console object specification just suggests "a space or something
similar", it does not explicitly require a space to be used).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
As the RTC stats "flood" the console output now the index of each group
of stats as well as its index in the group is printed to ease
differentiating them.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the add-more-information-to-peerconnectionanalyzer-logs branch from d848230 to f88e0fa Compare May 7, 2025 19:11
@danxuliu danxuliu merged commit 90c2d14 into main May 7, 2025
53 checks passed
@danxuliu danxuliu deleted the add-more-information-to-peerconnectionanalyzer-logs branch May 7, 2025 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants