New issue
Advanced search Search tips

Issue 811683 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 799030



Sign in to add a comment

WebRTC: Add UMA counters for Unified Plan

Project Member Reported by hta@chromium.org, Feb 13 2018

Issue description

In order to detect usage of Unified Plan, we need to count how much of it is occuring.

At a minimum, count media sections in:

- Answers accepted with a=msid lines only (Unified Plan)
- Answers accepted with a=ssrc msid lines only (Plan B)
- Answers accepted with both types of msid lines

The reason to count answers rather than offers is because offers should be mostly sent in "compatible mode" (both types of msid lines).

We should also count at PeerConnection creation:

- PCs created in "default mode"
- PCs created with "force Unified"
- PCs created with "force Plan B"

This is to keep an eye out for people trying the new way (before we switch) and for people needing the old way (after the switch).

 

Comment 1 by hta@chromium.org, Feb 13 2018

Blocking: 799030
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 14 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/5dbb58602f2456afd7ae1bd67e5851336dc725d1

commit 5dbb58602f2456afd7ae1bd67e5851336dc725d1
Author: Harald Alvestrand <hta@webrtc.org>
Date: Wed Feb 14 00:18:20 2018

Add UMA counters for type of SDP semantic in use.

We count a) what semantics are asked for explicitly (if any),
and b) what semantics are reflected in the successfully
processed answer, as indicated by presence of msid lines
of type Unified Plan vs Plan B.

This gives an indication of usage in sessions initiated by
the browser. It does not indicate usage in sessions where the
browser is the answerer.

Bug:  chromium:811683 
Change-Id: I2e28a6a83df1664e1aa1e17cd4ff2921de1fba7e
Reviewed-on: https://webrtc-review.googlesource.com/52101
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22008}
[modify] https://crrev.com/5dbb58602f2456afd7ae1bd67e5851336dc725d1/api/umametrics.h
[modify] https://crrev.com/5dbb58602f2456afd7ae1bd67e5851336dc725d1/pc/peerconnection.cc
[modify] https://crrev.com/5dbb58602f2456afd7ae1bd67e5851336dc725d1/pc/peerconnection_rtp_unittest.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 15 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5797c991e872591f4c99cc86f32fc2ad0e01481a

commit 5797c991e872591f4c99cc86f32fc2ad0e01481a
Author: Harald Alvestrand <hta@chromium.org>
Date: Thu Feb 15 11:42:08 2018

Wire up WebRTC SDP Semantics UMA counters

This exposes counters for what kind of SDP semantic
is requested and used to UMA.
Part of the WebRTC transition to Unified Plan.

Bug:  chromium:811683 
Change-Id: Idd306454e9ca2424612dc2fe5196e1ee77539df2
Reviewed-on: https://chromium-review.googlesource.com/918502
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Commit-Queue: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536992}
[modify] https://crrev.com/5797c991e872591f4c99cc86f32fc2ad0e01481a/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/5797c991e872591f4c99cc86f32fc2ad0e01481a/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/5797c991e872591f4c99cc86f32fc2ad0e01481a/tools/metrics/histograms/histograms.xml

Comment 4 by hta@chromium.org, Feb 19 2018

First results (one day of Canary) show 75% being counted as "none". This indicates that a lot of PCs get accepted with no MSID flags in setRemoteDescription(answer).
This may be one-way peerconnections, data-only peerconnections, or peerconnections with SFUs or MCUs that don't return MSIDs.

We should add counting setLocalDescription(answer), so that we get stats on the situations where Chrome is the answerer too.

Project Member

Comment 5 by bugdroid1@chromium.org, Feb 28 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/0ffaaa26f916796b3eac58063d56b0aa6e0321dc

commit 0ffaaa26f916796b3eac58063d56b0aa6e0321dc
Author: Steve Anton <steveanton@webrtc.org>
Date: Wed Feb 28 02:47:57 2018

Report negotiated SDP semantics for local answers also

Bug:  chromium:811683 
Change-Id: I8c51a3a1f58190c9dcd849ef451254ce230ea710
Reviewed-on: https://webrtc-review.googlesource.com/57128
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22216}
[modify] https://crrev.com/0ffaaa26f916796b3eac58063d56b0aa6e0321dc/pc/peerconnection.cc
[modify] https://crrev.com/0ffaaa26f916796b3eac58063d56b0aa6e0321dc/pc/peerconnection.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 6 2018

Labels: merge-merged-66
The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/9d75cf5f252e0a6cedc81c008db907ac14f32aea

commit 9d75cf5f252e0a6cedc81c008db907ac14f32aea
Author: Steve Anton <steveanton@webrtc.org>
Date: Tue Mar 06 18:15:53 2018

Report negotiated SDP semantics for local answers also

(cherry picked from commit 0ffaaa26f916796b3eac58063d56b0aa6e0321dc)

Bug:  chromium:811683 
Change-Id: I8c51a3a1f58190c9dcd849ef451254ce230ea710
Reviewed-on: https://webrtc-review.googlesource.com/57128
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Original-Commit-Position: refs/heads/master@{#22216}
Reviewed-on: https://webrtc-review.googlesource.com/59941
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/branch-heads/66@{#4}
Cr-Branched-From: 12c8110e8c717b7f0f87615d3b99caac2a69fa6c-refs/heads/master@{#22215}
[modify] https://crrev.com/9d75cf5f252e0a6cedc81c008db907ac14f32aea/pc/peerconnection.cc
[modify] https://crrev.com/9d75cf5f252e0a6cedc81c008db907ac14f32aea/pc/peerconnection.h

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 6 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/8e20f17d1d02bb084a91a099850f591d59ee873e

commit 8e20f17d1d02bb084a91a099850f591d59ee873e
Author: Steve Anton <steveanton@webrtc.org>
Date: Tue Mar 06 21:22:51 2018

Report UMA metrics for received SDP format

This change adds UMA stats that record the format of the remote offered
SDP. There are three classifications for the SDP format:
- Simple: No more than one audio and one video. Should be compatible
    with both Plan B and Unified Plan endpoints.
- ComplexPlanB: More than one audio or more than one video in the Plan B
    format (e.g., one audio mline with multiple streams).
- ComplexUnifiedPlan: More than one audio or more than one video in the
    Unified Plan format (e.g., two audio mlines).

Bug:  chromium:811683 
Change-Id: If46394edfa6a812ef313d632e27ec27516c18867
Reviewed-on: https://webrtc-review.googlesource.com/57220
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22315}
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/api/fakemetricsobserver.cc
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/api/fakemetricsobserver.h
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/api/umametrics.h
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/pc/peerconnection.cc
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/pc/peerconnection.h
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/pc/peerconnection_rtp_unittest.cc
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/pc/peerconnectionwrapper.cc
[modify] https://crrev.com/8e20f17d1d02bb084a91a099850f591d59ee873e/pc/peerconnectionwrapper.h

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f8df2253f2912ec76cdd1ed970cc913ad0ecb4e

commit 8f8df2253f2912ec76cdd1ed970cc913ad0ecb4e
Author: Steve Anton <steveanton@chromium.org>
Date: Thu Apr 05 20:46:36 2018

Add WebRTC SdpFormatReceived UMA counter

This was added to WebRTC in
https://webrtc-review.googlesource.com/c/src/+/57220

Bug:  811683 
Change-Id: I8a53ed68fd1b76ae222f82f3246120bba4ac3ee2
Reviewed-on: https://chromium-review.googlesource.com/998432
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Steve Anton <steveanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548547}
[modify] https://crrev.com/8f8df2253f2912ec76cdd1ed970cc913ad0ecb4e/content/renderer/media/webrtc/rtc_peer_connection_handler.cc
[modify] https://crrev.com/8f8df2253f2912ec76cdd1ed970cc913ad0ecb4e/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/8f8df2253f2912ec76cdd1ed970cc913ad0ecb4e/tools/metrics/histograms/histograms.xml

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 12 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af79e73089e2f9f822994723f5b95e6a70610023

commit af79e73089e2f9f822994723f5b95e6a70610023
Author: Steve Anton <steveanton@chromium.org>
Date: Thu Apr 12 19:28:52 2018

Report SdpSemanticRequested in Chromium PeerConnection

This moves the reporting for the SdpSemanticRequested UMA metric
to the PeerConnection handler directly and removes the usage of
the SdpSemantic::kDefault WebRTC value.

Bug:  811683 
Change-Id: Icecd551e40fb174eb21a5bd48943d5f8b522dd42
Reviewed-on: https://chromium-review.googlesource.com/944248
Commit-Queue: Steve Anton <steveanton@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550306}
[modify] https://crrev.com/af79e73089e2f9f822994723f5b95e6a70610023/content/renderer/media/webrtc/rtc_peer_connection_handler.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://webrtc.googlesource.com/src.git/+/3acffc3b16684f852a831257ab31991953f2e05c

commit 3acffc3b16684f852a831257ab31991953f2e05c
Author: Steve Anton <steveanton@webrtc.org>
Date: Fri Apr 13 17:03:08 2018

Remove SdpSemantics::kDefault

This adds confusion to the native API and is only needed for
Chromium UMA metrics, so the appropriate metrics have been moved
upstream and kDefault option removed.

Bug:  chromium:811683 
Change-Id: I666d7f7793765b8d6edcd99416c8b6c957766f00
Reviewed-on: https://webrtc-review.googlesource.com/59261
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22864}
[modify] https://crrev.com/3acffc3b16684f852a831257ab31991953f2e05c/api/peerconnectioninterface.h
[modify] https://crrev.com/3acffc3b16684f852a831257ab31991953f2e05c/pc/peerconnection.cc
[modify] https://crrev.com/3acffc3b16684f852a831257ab31991953f2e05c/pc/peerconnection_integrationtest.cc
[modify] https://crrev.com/3acffc3b16684f852a831257ab31991953f2e05c/sdk/objc/Framework/Classes/PeerConnection/RTCConfiguration.mm
[modify] https://crrev.com/3acffc3b16684f852a831257ab31991953f2e05c/sdk/objc/Framework/Headers/WebRTC/RTCConfiguration.h

Project Member

Comment 11 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c42eb293fc9fab232250d9550fc37f0677ede892

commit c42eb293fc9fab232250d9550fc37f0677ede892
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Apr 13 23:06:02 2018

Roll src/third_party/webrtc/ 3ef3bfc2a..3acffc3b1 (52 commits)

https://webrtc.googlesource.com/src.git/+log/3ef3bfc2aafa..3acffc3b1668

$ git log 3ef3bfc2a..3acffc3b1 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:811683 ,chromium:None,chromium:831093,chromium:None,chromium:None,chromium:b/77825904,chromium:None,chromium:None,chromium:None,chromium:831996,chromium:None,chromium:796112,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:827917


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng;master.tryserver.chromium.win:win-msvc-dbg
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ib12df502703e5c0d6c0d1dcb935f69f0166fa0be
Reviewed-on: https://chromium-review.googlesource.com/1012762
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#550776}
[modify] https://crrev.com/c42eb293fc9fab232250d9550fc37f0677ede892/DEPS

Project Member

Comment 12 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af79e73089e2f9f822994723f5b95e6a70610023

commit af79e73089e2f9f822994723f5b95e6a70610023
Author: Steve Anton <steveanton@chromium.org>
Date: Thu Apr 12 19:28:52 2018

Report SdpSemanticRequested in Chromium PeerConnection

This moves the reporting for the SdpSemanticRequested UMA metric
to the PeerConnection handler directly and removes the usage of
the SdpSemantic::kDefault WebRTC value.

Bug:  811683 
Change-Id: Icecd551e40fb174eb21a5bd48943d5f8b522dd42
Reviewed-on: https://chromium-review.googlesource.com/944248
Commit-Queue: Steve Anton <steveanton@chromium.org>
Reviewed-by: Henrik Boström <hbos@chromium.org>
Reviewed-by: Harald Alvestrand <hta@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550306}
[modify] https://crrev.com/af79e73089e2f9f822994723f5b95e6a70610023/content/renderer/media/webrtc/rtc_peer_connection_handler.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Apr 17 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c42eb293fc9fab232250d9550fc37f0677ede892

commit c42eb293fc9fab232250d9550fc37f0677ede892
Author: webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Date: Fri Apr 13 23:06:02 2018

Roll src/third_party/webrtc/ 3ef3bfc2a..3acffc3b1 (52 commits)

https://webrtc.googlesource.com/src.git/+log/3ef3bfc2aafa..3acffc3b1668

$ git log 3ef3bfc2a..3acffc3b1 --date=short --no-merges --format='%ad %ae %s'

Created with:
  roll-dep src/third_party/webrtc
BUG= chromium:811683 ,chromium:None,chromium:831093,chromium:None,chromium:None,chromium:b/77825904,chromium:None,chromium:None,chromium:None,chromium:831996,chromium:None,chromium:796112,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:None,chromium:827917


The AutoRoll server is located here: https://webrtc-chromium-roll.skia.org

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.


CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_chromium_archive_rel_ng;master.tryserver.chromium.mac:mac_chromium_archive_rel_ng;master.tryserver.chromium.win:win-msvc-dbg
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: Ib12df502703e5c0d6c0d1dcb935f69f0166fa0be
Reviewed-on: https://chromium-review.googlesource.com/1012762
Commit-Queue: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Reviewed-by: webrtc-chromium-autoroll <webrtc-chromium-autoroll@skia-buildbots.google.com.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#550776}
[modify] https://crrev.com/c42eb293fc9fab232250d9550fc37f0677ede892/DEPS

Assuming there is more work planned here?

Comment 15 by hta@chromium.org, May 29 2018

Status: Fixed (was: Assigned)
I think we're done here.
Components: Blink>WebRTC>PeerConnection
[bulk-edit: disregard if N/A] Can the owner please set milestone to this bug if applicable?

Comment 18 by hta@chromium.org, Jun 7 2018

Labels: M-66
Since this was merged back to 66, setting milestone to 66.
Not an user-visible change, so no need to announce it separately.

Sign in to add a comment