New issue
Advanced search Search tips

Issue 871668 link

Starred by 1 user
Status: Fixed
Owner:
hychao@chromium.org
Closed: Aug 16
Cc:
cindyb@chromium.org
vapier@chromium.org
cychiang@chromium.org
chromeos-audio-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 870321



Sign in to add a comment

CRAS: use libmetrics instead of calling metrics_client directly

Project Member Reported by hychao@chromium.org, Aug 7 
Copy from  issue 870321 :

CRAS is now calling 'metrics_client' to send data to UMA, and that requires some more system calls to be added to seccomp policy file.

Change CRAS to use libmetrics makes it simpler.

https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1163643 metrics: Add C wrapper for SendCrosEventToUMA
https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1163644 adhd: depends on chromeos-base/metrics
https://chromium-review.googlesource.com/c/chromiumos/third_party/adhd/+/1163645 CRAS: metrics - Use libmetrics instead of calling metrics_client

Comment 1 by hychao@chromium.org, Aug 7 

Per code review comments, we want to get libchrome's BASE_VER environment variable to link libmetrics-${BASE_VER}, but we don't want adhd to depend on libchrome packge, for easier deployment of adhd to builder images.

vapier@ suggested that we might want to split eclasses to get what we want.

Comment 2 by hychao@chromium.org, Aug 7 

One more CL
https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/1164820 Split libchrome BASE_VER management into libchrome-version.eclass
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/07f66e26c577c8afef14f9a058a88b4f0b616791

commit 07f66e26c577c8afef14f9a058a88b4f0b616791
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Fri Aug 10 12:08:32 2018

CRAS: metrics - Use libmetrics instead of calling metrics_client

This change simplifies the seccomp policy file, by reducing
execve and a few other system calls.

BUG= chromium:871668 
TEST=emerge adhd with chromeos-base/metrics side change,
verify events and samples can be sent to UMA.
CQ-DEPEND=CL:1163643,CL:1163644

Change-Id: I28290de0b9f670cdc5ad739823490061c0545f9a
Reviewed-on: https://chromium-review.googlesource.com/1163645
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/07f66e26c577c8afef14f9a058a88b4f0b616791/cras/src/common/cras_metrics.c
[modify] https://crrev.com/07f66e26c577c8afef14f9a058a88b4f0b616791/cras/configure.ac
[modify] https://crrev.com/07f66e26c577c8afef14f9a058a88b4f0b616791/cras/src/Makefile.am
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/071ecf509efdf080be28b84ba3982c9c2f734aee

commit 071ecf509efdf080be28b84ba3982c9c2f734aee
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Fri Aug 10 12:08:31 2018

adhd: depends on chromeos-base/metrics

CRAS will use libmetrics to send data to UMA, instead
of directly calling metrics_client.

BUG= chromium:871668 
TEST=Apply with CLs from adhd and metrics, build and verify
events and samples can send to UMA.

Change-Id: I886c38f2735918119ebb9b2ac429f4efd1d8c82a
Reviewed-on: https://chromium-review.googlesource.com/1163644
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/071ecf509efdf080be28b84ba3982c9c2f734aee/media-sound/adhd/adhd-9999.ebuild
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/919aec2595f2aae2b3b4c444466e22a235867246

commit 919aec2595f2aae2b3b4c444466e22a235867246
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Fri Aug 10 12:08:31 2018

Split libchrome BASE_VER management into libchrome-version.eclass

This change allows a package to have libchrome BASE_VER environment
variable define, but not getting chromeos-base/libchrome in dependency.

BUG= chromium:871668 
TEST=Let media-sound/adhd inherit libchrome-version
equery -g adhd to make sure libchrome is not in dependency.

Change-Id: Ibeef3e8c07850766af03daa46a4d097d8826571b
Reviewed-on: https://chromium-review.googlesource.com/1164820
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[add] https://crrev.com/919aec2595f2aae2b3b4c444466e22a235867246/eclass/libchrome-version.eclass
[modify] https://crrev.com/919aec2595f2aae2b3b4c444466e22a235867246/eclass/libchrome.eclass
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/4b8fa88715c25e57009a3602da439830cf735b5e

commit 4b8fa88715c25e57009a3602da439830cf735b5e
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Fri Aug 10 12:08:30 2018

metrics: Add C wrapper for SendCrosEventToUMA

BUG= chromium:871668 
TEST=Build and test with CRAS linking libmetrics
and use the new API.

Change-Id: Iee0e90173b2f38ce434bf029f8aa32b292e4d3d8
Reviewed-on: https://chromium-review.googlesource.com/1163643
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

[modify] https://crrev.com/4b8fa88715c25e57009a3602da439830cf735b5e/metrics/c_metrics_library.cc
[modify] https://crrev.com/4b8fa88715c25e57009a3602da439830cf735b5e/metrics/c_metrics_library.h

Comment 7 by lhchavez@chromium.org, Aug 10

Components: OS>Kernel>Audio
Summary: CRAS: use libmetrics instead of calling metrics_client directly (was: CRAAS: use libmetrics instead of calling metrics_client directly)

Comment 8 by hychao@chromium.org, Aug 15

Cc: cindyb@chromium.org
Labels: Merge-Request-69
Request merge to M69
Project Member

Comment 9 by sheriffbot@chromium.org, Aug 15

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 10 by cindyb@chromium.org, Aug 15

Labels: -Merge-Review-69 Merge-Approved-69
Merge approved, M69.
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 16

Labels: merge-merged-release-R69-10895.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/415ecbeba41f4d1fe631000e3e4b414cb8a2bf3e

commit 415ecbeba41f4d1fe631000e3e4b414cb8a2bf3e
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Thu Aug 16 03:40:45 2018

metrics: Add C wrapper for SendCrosEventToUMA

BUG= chromium:871668 
TEST=Build and test with CRAS linking libmetrics
and use the new API.

Change-Id: Iee0e90173b2f38ce434bf029f8aa32b292e4d3d8
Reviewed-on: https://chromium-review.googlesource.com/1163643
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 4b8fa88715c25e57009a3602da439830cf735b5e)
Reviewed-on: https://chromium-review.googlesource.com/1177041
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/415ecbeba41f4d1fe631000e3e4b414cb8a2bf3e/metrics/c_metrics_library.cc
[modify] https://crrev.com/415ecbeba41f4d1fe631000e3e4b414cb8a2bf3e/metrics/c_metrics_library.h
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/04e7235e4e6a8db8f55d734b0b13871324499632

commit 04e7235e4e6a8db8f55d734b0b13871324499632
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Thu Aug 16 03:52:50 2018

Split libchrome BASE_VER management into libchrome-version.eclass

This change allows a package to have libchrome BASE_VER environment
variable define, but not getting chromeos-base/libchrome in dependency.

BUG= chromium:871668 
TEST=Let media-sound/adhd inherit libchrome-version
equery -g adhd to make sure libchrome is not in dependency.

Change-Id: Ibeef3e8c07850766af03daa46a4d097d8826571b
Reviewed-on: https://chromium-review.googlesource.com/1164820
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 919aec2595f2aae2b3b4c444466e22a235867246)
Reviewed-on: https://chromium-review.googlesource.com/1177101
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[add] https://crrev.com/04e7235e4e6a8db8f55d734b0b13871324499632/eclass/libchrome-version.eclass
[modify] https://crrev.com/04e7235e4e6a8db8f55d734b0b13871324499632/eclass/libchrome.eclass
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/d2c8d4b26efc1f2bdc3f6654fb0ce531bbed5364

commit d2c8d4b26efc1f2bdc3f6654fb0ce531bbed5364
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Thu Aug 16 04:10:13 2018

adhd: depends on chromeos-base/metrics

CRAS will use libmetrics to send data to UMA, instead
of directly calling metrics_client.

BUG= chromium:871668 
TEST=Apply with CLs from adhd and metrics, build and verify
events and samples can send to UMA.

Change-Id: I886c38f2735918119ebb9b2ac429f4efd1d8c82a
Reviewed-on: https://chromium-review.googlesource.com/1163644
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>
(cherry picked from commit 071ecf509efdf080be28b84ba3982c9c2f734aee)
Reviewed-on: https://chromium-review.googlesource.com/1177102
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/d2c8d4b26efc1f2bdc3f6654fb0ce531bbed5364/media-sound/adhd/adhd-9999.ebuild
Project Member

Comment 14 by bugdroid1@chromium.org, Aug 16 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/adhd/+/7033d4097d16c98bebbf64ad348f1b7d34601c5e

commit 7033d4097d16c98bebbf64ad348f1b7d34601c5e
Author: Hsin-Yu Chao <hychao@chromium.org>
Date: Thu Aug 16 04:25:34 2018

CRAS: metrics - Use libmetrics instead of calling metrics_client

This change simplifies the seccomp policy file, by reducing
execve and a few other system calls.

BUG= chromium:871668 
TEST=emerge adhd with chromeos-base/metrics side change,
verify events and samples can be sent to UMA.
CQ-DEPEND=CL:1163643,CL:1163644

Change-Id: I28290de0b9f670cdc5ad739823490061c0545f9a
Reviewed-on: https://chromium-review.googlesource.com/1163645
Commit-Ready: Hsinyu Chao <hychao@chromium.org>
Tested-by: Hsinyu Chao <hychao@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
(cherry picked from commit 07f66e26c577c8afef14f9a058a88b4f0b616791)
Reviewed-on: https://chromium-review.googlesource.com/1177090
Reviewed-by: Hsinyu Chao <hychao@chromium.org>
Commit-Queue: Hsinyu Chao <hychao@chromium.org>

[modify] https://crrev.com/7033d4097d16c98bebbf64ad348f1b7d34601c5e/cras/src/common/cras_metrics.c
[modify] https://crrev.com/7033d4097d16c98bebbf64ad348f1b7d34601c5e/cras/configure.ac
[modify] https://crrev.com/7033d4097d16c98bebbf64ad348f1b7d34601c5e/cras/src/Makefile.am

Comment 15 by hychao@chromium.org, Aug 16

Status: Fixed (was: Started)

Comment 16 by hychao@chromium.org, Aug 17

Labels: -Merge-Approved-69
►

Sign in to add a comment