[MIR] ubuntu-report: send telemetry data to ubuntu server

Bug #1759540 reported by Didier Roche-Tolomelli
14
This bug affects 1 person
Affects Status Importance Assigned to Milestone
ubuntu-report (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

Note that only the C library ("libsysmetrics1") and the command line ("ubuntu-report") packages are needed in main (we should also promote "libsysmetrics-dev" package, even if not necessary). We don't need the Go API "golang-github-ubuntu-ubuntu-report-dev" which has many go package dependencies in universe.

[Availability]
Available and built on all archs.

[Rationale]
We want to provide telemetry upload features, collected by ubiquity, do-release-upgrader and that package in ubnutu 18.04. see https://bugs.launchpad.net/ubuntu/+source/ubiquity/+bug/1755456 for more rationales and details.

[Security]
No security issue reported, as brand new package.
The package do a POST message over https to an ubuntu server, data being sent are really limited to garantee anynomous informations.
See example of what is sent on https://github.com/ubuntu/ubuntu-report README.

[Quality assurance]
The canonical desktop team is the upstream maintainer and will continue maintening it.
Upstream has CI built-in on every commit + coverage report. The package build and runs (currently) 243 tests from unit to integration tests. Autopkgtests ensuring that building works and tests still pass is also provided.

In addition the documentation and C API are continously strongly tested (with examples extracted from the documentation, built, linked against a on-demand generated C library, and tests ran against it).

[Dependencies]
Upstream is using a vendor/ directory, but the ubuntu package remove it on build and use the distribution packaged build-deps equivalent.
No runtime dep in the 3 packages that needs to be promoted.

[Standards compliance]
Package follow latest debian packaging standard compliant. No lintian override files, and binaries/packages follows Debian standard FHS. Note that there is no .symbols file as the generated C API export internal symbols (starting with _) which names contains a hash dependant of the source file, and per arch specific. The exported symbols from the public API have a stable name though: https://godoc.org/github.com/ubuntu/ubuntu-report/pkg/sysmetrics/C

[Maintenance]
Canonical desktop team being the usptream and downstream maintainer, strongly backed by tests, will follow bugs and maintenance.

description: updated
description: updated
description: updated
Revision history for this message
Matthias Klose (doko) wrote :

 - libsysmetrics1 is missing a symbols file
 - Package description s/contain/contains/
 - the copyrights are incomplete. grep -ri copyright shows a lot more names.
   that includes the vendor files, but these are shipped in the source package.

Changed in ubuntu-report (Ubuntu):
status: New → Incomplete
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Doko asked me to look at the package and it seems OK to me on a quick glance. (Only spent a few minutes, let me know if you'd like a deeper review!)

On the packaging side, I'm a bit surprised at the use of go:generate to create the shared lib but it works I guess. Another thing that's more on the taste side is that rather than knowing that debhelper puts build artefacts in obj-$(DEB_HOST_GNU_TYPE), I prefer to pass --builddirectory=_build or whatever to dh.

I'd generally be happier if the vendor directory wasn't in the source tarball at all (which would also get you out of updating debian/copyright :-p). I think you can do this by putting tar-ignore=ubuntu-report/vendor into debian/source/options? (Side comment: why is debian/ in .gitignore??)

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

@doko:
- the symbols file would be irrelevant. We tried it in previous uploads (see package), but any even non API breaking symbols generates different hash for internal symbols which starts with _ (and per architecture…). Would you have a way to tell dh_symbols to ignore all symbols starting with "_"?
- typo fixed
- I think I'll go with Michael's approach of not shipping the vendor directory in the source tarball at all, that will be quicker and easier to test.

@Michael:
- the idea was to have an easy builded way of generating the shared libs which can be used in CI and such without repeating documentation from the source code.
- ok on stripping the vendor directory from the source package. I added whole debian/ in gitignore to not pick new artfacts in the Vcs and only git adding what was desired.

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Ok, uploaded 1.0.6 which address the above issue, stripping the vendor directory from the source tarball (and change debian/copyright), typo fixes, while shipping a symbols file which ignores private cgo symbols.

Please let me know if any other changes are required, thanks!

Changed in ubuntu-report (Ubuntu):
status: Incomplete → New
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

The package looks fine to me now.

Revision history for this message
Sebastien Bacher (seb128) wrote :

The reviews state that the package is fine, the few tweaks that have been requested have been done, setting as fix commited

Changed in ubuntu-report (Ubuntu):
status: New → Fix Committed
Revision history for this message
Steve Langasek (vorlon) wrote :

Override component to main
ubuntu-report 1.0.7 in bionic: universe/utils -> main
1 publication overridden.
Override component to main
ubuntu-report 1.0.7 in bionic amd64: universe/utils/optional/100% -> main
ubuntu-report 1.0.7 in bionic arm64: universe/utils/optional/100% -> main
ubuntu-report 1.0.7 in bionic armhf: universe/utils/optional/100% -> main
ubuntu-report 1.0.7 in bionic i386: universe/utils/optional/100% -> main
ubuntu-report 1.0.7 in bionic ppc64el: universe/utils/optional/100% -> main
ubuntu-report 1.0.7 in bionic s390x: universe/utils/optional/100% -> main
libsysmetrics1 1.0.7 in bionic amd64: universe/libs/optional/100% -> main
libsysmetrics1 1.0.7 in bionic arm64: universe/libs/optional/100% -> main
libsysmetrics1 1.0.7 in bionic armhf: universe/libs/optional/100% -> main
libsysmetrics1 1.0.7 in bionic i386: universe/libs/optional/100% -> main
libsysmetrics1 1.0.7 in bionic ppc64el: universe/libs/optional/100% -> main
libsysmetrics1 1.0.7 in bionic s390x: universe/libs/optional/100% -> main
libsysmetrics-dev 1.0.7 in bionic amd64: universe/devel/optional/100% -> main
libsysmetrics-dev 1.0.7 in bionic arm64: universe/devel/optional/100% -> main
libsysmetrics-dev 1.0.7 in bionic armhf: universe/devel/optional/100% -> main
libsysmetrics-dev 1.0.7 in bionic i386: universe/devel/optional/100% -> main
libsysmetrics-dev 1.0.7 in bionic ppc64el: universe/devel/optional/100% -> main
libsysmetrics-dev 1.0.7 in bionic s390x: universe/devel/optional/100% -> main
18 publications overridden.

Changed in ubuntu-report (Ubuntu):
status: Fix Committed → Fix Released
Revision history for this message
Steve Langasek (vorlon) wrote :

> We don't need the Go API "golang-github-ubuntu-ubuntu-report-dev"
> which has many go package dependencies in universe.

Unfortunately, listing golang-github-ubuntu-ubuntu-report-dev is not sufficient to keep the go tree out of main. ubuntu-report is itself implemented in go, which means it statically links all the various go libraries that it uses. The closure of main traverses Depends, Recommends, and also Built-Using - specifically to ensure that statically-linked code included in a binary in main doesn't go unnoticed and missed wrt security support.

So there is in fact a tremendous dependency tree that you would need to MIR in order to ship ubuntu-report in main, as shown at <http://people.canonical.com/~ubuntu-archive/component-mismatches.svg>. I don't think it's realistic to have these dependencies addressed in time for release, and would recommend withdrawing this MIR for 18.04 (and unseeding the package).

Changed in ubuntu-report (Ubuntu):
status: Fix Released → Incomplete
Revision history for this message
Sebastien Bacher (seb128) wrote :

The issue got discussed on IRC, as written in the description the upstream projects include a vendors/ subdir with those components, we didn't use that because we though using the distribution versions was the preferred way. We are going to switch back to use the vendors/ copies instead (which e.g snapd is doing), could someone from the MIR team review the source again with that in mind and confirm it's ok?
The upstream source including the vendors bundle can be found on https://github.com/ubuntu/ubuntu-report

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

As discussed, ubuntu-report is now using its own upstream vendored dependencies corresponding to the ancient build-deps here. This is what upstream is already testing against.
debian/copyright is updated (grepping manually for copyrights, as per Matthias' remark) and build system updated in ubuntu-report 1.0.9.

As nesting vendor isn't something that Go is going to support for long and isn't part of best practice, I kept the deps listing on the "golang-github-ubuntu-ubuntu-report-dev" package so that golang devs have them available and ensure the vendor isn't shipped in that source package. Note thus that "golang-github-ubuntu-ubuntu-report-dev" should still be kept in main.

I think there is some work needed to ensure that this is written down in the MIR requirement so that both MIR team and requesters are aware about it.
Also, the check-mir tool needs update to cope with that case.

Changed in ubuntu-report (Ubuntu):
status: Incomplete → New
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Reverting to "NEW" for second review (1.0.9) from the MIR team.

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

"Note thus that "golang-github-ubuntu-ubuntu-report-dev" should still be kept in main." -> "Note thus that "golang-github-ubuntu-ubuntu-report-dev" should still be kept in **universe**." of course.

Revision history for this message
Sebastien Bacher (seb128) wrote :

that one was promoted in bionic

Changed in ubuntu-report (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.