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

Issue 773487 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[Media Router] Cast discovery type metrics seem to be missing

Project Member Reported by mfo...@chromium.org, Oct 10 2017

Issue description

UMA knows about the following histograms:

1. MediaRouter.Cast.Discovery.SinkSource
owner: btolsch@chromium.org

=> Currently no references in code I can find.

2. MediaRouter.Cast.Discovery.Type
owner: cloudview-eng@

=> Seems to be collected by the component extension.

Meanwhile media_sink_discovery_metrics.cc references the following histogram:

3. MediaRouter.Cast.Discovery.CachedSinkResolved

=> Not defined in UMA.

=====

It seems like we should replace references to #3 with #1, and deprecate and remove #2 when we remove discovery code from the component.  Does that sound right?

 

Comment 1 by mfo...@chromium.org, Oct 10 2017

Cc: imch...@chromium.org
Owner: mfo...@chromium.org
Status: Assigned (was: Untriaged)
Yeah, looks like a mix-up of names of (1) and (3). The metric is recorded here. https://cs.chromium.org/chromium/src/chrome/browser/media/router/discovery/media_sink_discovery_metrics.cc?type=cs&sq=package:chromium&l=73 This should be renamed MediaRouter.Cast.Discovery.SinkSource and added to histograms.xml.

Re (2): Yes the plan is to deprecate it and replace it with (1) MediaRouter.Cast.Discovery.SinkSource which is a bit more comprehensive. The dashboard will need to be updated with the new histogram.


Comment 3 by mfo...@chromium.org, Oct 12 2017

Status: Started (was: Assigned)

Comment 4 by mfo...@chromium.org, Oct 12 2017

Labels: Hotlist-CodeHealth
Also, the following should have their definitions added to the Chromium histograms file, if possible:

MediaRouter.Cast.Discovery.ConnectedDevicesCount
MediaRouter.Cast.Discovery.KnownDevicesCount
MediaRouter.Dial.KnownDevicesCount
MediaRouter.Dial.AvailableDevicesCount
MediaRouter.Cast.Mdns.Channel.Open  (How is this different from the one below?)
MediaRouter.Cast.Channel.ConnectResult
MediaRouter.Cast.Channel.Error

Since they are now collected in Chromium code.
MediaRouter.Cast.Mdns.Channel.Open_xxx // result for one attempt
MediaRouter.Cast.Channel.ConnectResult // result after all retry attempts

Comment 6 by mfo...@chromium.org, Oct 12 2017

If they already exist in the internal histograms file, the internal definitions can be obsoleted or removed.  Have a question out to the team on the details.
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 12 2017

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

commit 5e3220addfdff5447450010f135d09df7915c4dd
Author: mark a. foltz <mfoltz@chromium.org>
Date: Thu Oct 12 20:54:37 2017

[Media Router] Use the correct histogram to record sink source.

The incorrect histogram name was being used to record the discovery source for
Cast devices.  This fixes that and renames the related method accordingly.

Bug:  773487 
Change-Id: Ib1ca58f7071eda4ccaa87500bb416a10d324634f
Reviewed-on: https://chromium-review.googlesource.com/717022
Reviewed-by: Bin Zhao <zhaobin@chromium.org>
Commit-Queue: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508456}
[modify] https://crrev.com/5e3220addfdff5447450010f135d09df7915c4dd/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl.cc
[modify] https://crrev.com/5e3220addfdff5447450010f135d09df7915c4dd/chrome/browser/media/router/discovery/mdns/cast_media_sink_service_impl_unittest.cc
[modify] https://crrev.com/5e3220addfdff5447450010f135d09df7915c4dd/chrome/browser/media/router/discovery/media_sink_discovery_metrics.cc
[modify] https://crrev.com/5e3220addfdff5447450010f135d09df7915c4dd/chrome/browser/media/router/discovery/media_sink_discovery_metrics.h

Comment 8 by mfo...@chromium.org, Oct 13 2017

Status: Fixed (was: Started)
Looks like the Chromium histograms.xml is already up to date (thanks zhaobin@).

Sign in to add a comment