New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 914452 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit 19 days ago
Closed: Dec 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

Add UMA metrics for WebRTC mDNS candidate use

Project Member Reported by jeroendb@chromium.org, Dec 12

Issue description

In WebRTC a new type of host candidate has been introduced.

To track it's effectiveness and impact on overall WebRTC connectivity, some values have been added to the existing "IceCandidatePairTypes" enum metric.

These changes are tracking in webRTC with:

https://webrtc-review.googlesource.com/c/src/+/113321/10 and
https://webrtc-review.googlesource.com/c/src/+/113643

Both are in review and expected to be submitted to webRTC today or tomorrow latest.

A separate chromium CL is needed to add labels (to tools/histograms/enum.xml) for the new enum values.

 
Cc: qingsi@chromium.org emadomara@chromium.org
Components: Blink>WebRTC>Network
Project Member

Comment 2 by bugdroid1@chromium.org, Dec 13

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

commit 1dac6d8839a227d3b5c1418db77e9f712545fd26
Author: Qingsi Wang <qingsi@google.com>
Date: Thu Dec 13 00:27:33 2018

Sanitize candidates in ICE-level stats when necessary.

The address and the related address of local candidates are sanitized
accordingly when the mDNS concealment of local IPs is enabled. Also,
remote hostname candidates created from signaling are sanitized in stats
as well. A couple of unit tests are revised to reflect the desired
behavior of AsyncResolverInterface so that when a hostname candidate is
resolved, the hostname is kept in the candidate address.

Bug:  webrtc:9605 ,  chromium:914452 
Change-Id: Iad9ad04ce4e50304e44cf04b15b97a7ae2dec960
Reviewed-on: https://webrtc-review.googlesource.com/c/113643
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Jeroen de Borst <jeroendb@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25996}
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/api/candidate.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/api/candidate.h
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/p2p/base/p2ptransportchannel_unittest.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/p2p/base/port.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/p2p/base/port.h
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/p2p/base/portallocator.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/p2p/client/basicportallocator.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/p2p/client/basicportallocator_unittest.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/pc/peerconnection.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/pc/peerconnection_integrationtest.cc
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/rtc_base/fake_mdns_responder.h
[modify] https://crrev.com/1dac6d8839a227d3b5c1418db77e9f712545fd26/rtc_base/fakenetwork.h

Project Member

Comment 3 by bugdroid1@chromium.org, Dec 13

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

commit 833979f7b8a43d7ed41e660dcfc63023d266c3fb
Author: Jeroen de Borst <jeroendb@webrtc.org>
Date: Thu Dec 13 17:35:10 2018

Adding metrics for hostname candidate use.

These metrics by themselves won't be as useful, unless they can be correlated to the use of the
feature 'WebRtcHideLocalIpsWithMdns'. This can be done by running a finch experiment where we turn
the feature on for a % of users, we can then compare these metrics for users with and without
the feature turned on.

A complementary change is required in Chrome:
tools/metrics/histograms/enums.xml

Bug:  webrtc:9605  webrtc:10091  chromium:914452 
Change-Id: Ibc6d16dec95a8e3943ce40063c02903769fe1cb4
Reviewed-on: https://webrtc-review.googlesource.com/c/113321
Commit-Queue: Jeroen de Borst <jeroendb@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26003}
[modify] https://crrev.com/833979f7b8a43d7ed41e660dcfc63023d266c3fb/api/umametrics.h
[modify] https://crrev.com/833979f7b8a43d7ed41e660dcfc63023d266c3fb/pc/peerconnection.cc
[modify] https://crrev.com/833979f7b8a43d7ed41e660dcfc63023d266c3fb/pc/peerconnection_integrationtest.cc

Cc: gov...@chromium.org
The WebRTC changes have landed.


Cc: abdulsyed@chromium.org srinivassista@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Dec 13

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

commit 7fc3ced0cb5e09efc3a73688ee2488b0a3fbccec
Author: Jeroen de Borst <jeroendb@chromium.org>
Date: Thu Dec 13 22:06:19 2018

Add UMA labels to match WebRTC enum

To be submitted when to https://webrtc-review.googlesource.com/c/src/+/113321 is imported in Chrome.

Bug:  chromium:914452 
Change-Id: Ibea51d0e41687bd775085fa7198cc59d528e8eb7
Reviewed-on: https://chromium-review.googlesource.com/c/1374744
Reviewed-by: Steve Anton <steveanton@chromium.org>
Commit-Queue: Jeroen de Borst <jeroendb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616450}
[modify] https://crrev.com/7fc3ced0cb5e09efc3a73688ee2488b0a3fbccec/tools/metrics/histograms/enums.xml

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 14

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

commit adcf3bbef5b8888f9b069eca90164855d8cc85a0
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Fri Dec 14 01:10:10 2018

Roll src/third_party/webrtc f7f13e0742a9..d96b275cd686 (9 commits)

https://webrtc.googlesource.com/src.git/+log/f7f13e0742a9..d96b275cd686


git log f7f13e0742a9..d96b275cd686 --date=short --no-merges --format='%ad %ae %s'
2018-12-13 sprang@webrtc.org Refactor EncodeParameters usage, remove unused rtt/loss
2018-12-13 henrikg@webrtc.org Revert "Implement read-only codecPayloadType in RtpParameters"
2018-12-13 magjed@webrtc.org Android: One weird trick for avoiding graphics deadlocks
2018-12-13 oprypin@webrtc.org Increase timeout of webrtc_perf_tests on Android to 1h15m
2018-12-13 ouj@fb.com Create field trial for vp8 number of thread on iOS.
2018-12-13 qingsi@google.com Sanitize candidates in ICE-level stats when necessary.
2018-12-12 chromium-webrtc-autoroll@webrtc-ci.iam.gserviceaccount.com Roll chromium_revision 23962c3887..55b877610b (615838:615952)
2018-12-12 srte@webrtc.org Cleanup of RtpTransportControllerSend.
2018-12-12 orphis@webrtc.org Implement read-only codecPayloadType in RtpParameters


Created with:
  gclient setdep -r src/third_party/webrtc@d96b275cd686

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

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=luci.chromium.try:linux_chromium_archive_rel_ng;luci.chromium.try:mac_chromium_archive_rel_ng

BUG=chromium:b/120481228,chromium:914452,chromium:None
TBR=webrtc-chromium-sheriffs-robots@google.com

Change-Id: If31091bccfac5903d85454cd9dab21b1e5b867e5
Reviewed-on: https://chromium-review.googlesource.com/c/1375401
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#616540}
[modify] https://crrev.com/adcf3bbef5b8888f9b069eca90164855d8cc85a0/DEPS

Labels: Merge-Approved-72
Approving merge to M72 branch 3626 based on internal email thread.
Project Member

Comment 9 by bugdroid1@chromium.org, Dec 15

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2772ad706934ab3290206a3b7f8ed5a0b0697dd0

commit 2772ad706934ab3290206a3b7f8ed5a0b0697dd0
Author: Jeroen de Borst <jeroendb@chromium.org>
Date: Sat Dec 15 00:24:57 2018

Merge to M72: Add UMA labels to match WebRTC enum

(cherry picked from commit 7fc3ced0cb5e09efc3a73688ee2488b0a3fbccec)

TBR=steveanton@chromium.org

Bug:  chromium:914452 
Change-Id: Ibea51d0e41687bd775085fa7198cc59d528e8eb7
Reviewed-on: https://chromium-review.googlesource.com/c/1374744
Reviewed-by: Steve Anton <steveanton@chromium.org>
Commit-Queue: Jeroen de Borst <jeroendb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616450}
Reviewed-on: https://chromium-review.googlesource.com/c/1379226
Cr-Commit-Position: refs/branch-heads/3626@{#378}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/2772ad706934ab3290206a3b7f8ed5a0b0697dd0/tools/metrics/histograms/enums.xml

Status: Fixed (was: Assigned)
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/2772ad706934ab3290206a3b7f8ed5a0b0697dd0

Commit: 2772ad706934ab3290206a3b7f8ed5a0b0697dd0
Author: jeroendb@chromium.org
Commiter: steveanton@chromium.org
Date: 2018-12-15 00:24:57 +0000 UTC

Merge to M72: Add UMA labels to match WebRTC enum

(cherry picked from commit 7fc3ced0cb5e09efc3a73688ee2488b0a3fbccec)

TBR=steveanton@chromium.org

Bug:  chromium:914452 
Change-Id: Ibea51d0e41687bd775085fa7198cc59d528e8eb7
Reviewed-on: https://chromium-review.googlesource.com/c/1374744
Reviewed-by: Steve Anton <steveanton@chromium.org>
Commit-Queue: Jeroen de Borst <jeroendb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#616450}
Reviewed-on: https://chromium-review.googlesource.com/c/1379226
Cr-Commit-Position: refs/branch-heads/3626@{#378}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment