Patchwork [WIP,PATCHv2] report-error: Allow to upload reports automatically

login
register
mail settings
Submitter Martin Jansa
Date March 17, 2014, 9:25 a.m.
Message ID <1395048338-25754-1-git-send-email-Martin.Jansa@gmail.com>
Download mbox | patch
Permalink /patch/68735/
State New
Headers show

Comments

Martin Jansa - March 17, 2014, 9:25 a.m.
* useful when distro wants to collect build statistics from
  all users/developers without any manual interaction from them

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/classes/report-error.bbclass    | 103 ++++++++++++++++++++++++++++++++++-
 meta/conf/local.conf.sample.extended |  20 +++++++
 2 files changed, 121 insertions(+), 2 deletions(-)
Richard Purdie - March 17, 2014, 10:38 a.m.
On Mon, 2014-03-17 at 10:25 +0100, Martin Jansa wrote:
> * useful when distro wants to collect build statistics from
>   all users/developers without any manual interaction from them
> 
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/classes/report-error.bbclass    | 103 ++++++++++++++++++++++++++++++++++-
>  meta/conf/local.conf.sample.extended |  20 +++++++
>  2 files changed, 121 insertions(+), 2 deletions(-)

I get worried about this as people could (rightly) be very nervous about
the system sharing information with a remote server without their
knowledge. Adding in this functionality makes it look as if that is the
intent of the system.

This is why there was a two step process, first to save out the data and
second to have the user upload it (once they'd looked at what it was
sharing).

Does anyone else have thoughts on this?

Cheers,

Richard
Paul Eggleton - March 17, 2014, 11:03 a.m.
On Monday 17 March 2014 10:38:05 Richard Purdie wrote:
> On Mon, 2014-03-17 at 10:25 +0100, Martin Jansa wrote:
> > * useful when distro wants to collect build statistics from
> > 
> >   all users/developers without any manual interaction from them
> > 
> > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > ---
> > 
> >  meta/classes/report-error.bbclass    | 103
> >  ++++++++++++++++++++++++++++++++++- meta/conf/local.conf.sample.extended
> >  |  20 +++++++
> >  2 files changed, 121 insertions(+), 2 deletions(-)
> 
> I get worried about this as people could (rightly) be very nervous about
> the system sharing information with a remote server without their
> knowledge. Adding in this functionality makes it look as if that is the
> intent of the system.
> 
> This is why there was a two step process, first to save out the data and
> second to have the user upload it (once they'd looked at what it was
> sharing).
> 
> Does anyone else have thoughts on this?

I have the same concerns. It's fine if people enable this in their own build 
server's local.conf / auto.conf; but if someone were to put this into their 
distro config which is downloaded and used by someone to do their own builds 
with configuration or additional components that they do not wish to share, 
there's the potential for that person to be unaware that their build failures 
will be uploaded and shared automatically.

Cheers,
Paul
Martin Jansa - March 30, 2014, 11:25 a.m.
On Mon, Mar 17, 2014 at 11:03:06AM +0000, Paul Eggleton wrote:
> On Monday 17 March 2014 10:38:05 Richard Purdie wrote:
> > On Mon, 2014-03-17 at 10:25 +0100, Martin Jansa wrote:
> > > * useful when distro wants to collect build statistics from
> > > 
> > >   all users/developers without any manual interaction from them
> > > 
> > > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > > ---
> > > 
> > >  meta/classes/report-error.bbclass    | 103
> > >  ++++++++++++++++++++++++++++++++++- meta/conf/local.conf.sample.extended
> > >  |  20 +++++++
> > >  2 files changed, 121 insertions(+), 2 deletions(-)
> > 
> > I get worried about this as people could (rightly) be very nervous about
> > the system sharing information with a remote server without their
> > knowledge. Adding in this functionality makes it look as if that is the
> > intent of the system.
> > 
> > This is why there was a two step process, first to save out the data and
> > second to have the user upload it (once they'd looked at what it was
> > sharing).
> > 
> > Does anyone else have thoughts on this?
> 
> I have the same concerns. It's fine if people enable this in their own build 
> server's local.conf / auto.conf; but if someone were to put this into their 
> distro config which is downloaded and used by someone to do their own builds 
> with configuration or additional components that they do not wish to share, 
> there's the potential for that person to be unaware that their build failures 
> will be uploaded and shared automatically.

My use-case was private distro, where it's imho valid to gather
build stats from every build which happens in the company (we're using
it to find builds which weren't configured correctly, e.g. referencing
non-existent premirror/sstate-mirror etc).

Should I just change the description and commit message so that it
doesn't promote it for distro config to use it or should I keep it all
private?

I'm thinking about using this also in my bitbake world builds and it's
also easier to enable it in config there, than parsing the log and
calling send-error-report from jenkins job config when report was
created.

Should I use error-report-web installed on yoctoproject.org or create my own
instance somewhere?
Richard Purdie - March 30, 2014, 12:35 p.m.
On Sun, 2014-03-30 at 13:25 +0200, Martin Jansa wrote:
> On Mon, Mar 17, 2014 at 11:03:06AM +0000, Paul Eggleton wrote:
> > On Monday 17 March 2014 10:38:05 Richard Purdie wrote:
> > > On Mon, 2014-03-17 at 10:25 +0100, Martin Jansa wrote:
> > > > * useful when distro wants to collect build statistics from
> > > > 
> > > >   all users/developers without any manual interaction from them
> > > > 
> > > > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > > > ---
> > > > 
> > > >  meta/classes/report-error.bbclass    | 103
> > > >  ++++++++++++++++++++++++++++++++++- meta/conf/local.conf.sample.extended
> > > >  |  20 +++++++
> > > >  2 files changed, 121 insertions(+), 2 deletions(-)
> > > 
> > > I get worried about this as people could (rightly) be very nervous about
> > > the system sharing information with a remote server without their
> > > knowledge. Adding in this functionality makes it look as if that is the
> > > intent of the system.
> > > 
> > > This is why there was a two step process, first to save out the data and
> > > second to have the user upload it (once they'd looked at what it was
> > > sharing).
> > > 
> > > Does anyone else have thoughts on this?
> > 
> > I have the same concerns. It's fine if people enable this in their own build 
> > server's local.conf / auto.conf; but if someone were to put this into their 
> > distro config which is downloaded and used by someone to do their own builds 
> > with configuration or additional components that they do not wish to share, 
> > there's the potential for that person to be unaware that their build failures 
> > will be uploaded and shared automatically.
> 
> My use-case was private distro, where it's imho valid to gather
> build stats from every build which happens in the company (we're using
> it to find builds which weren't configured correctly, e.g. referencing
> non-existent premirror/sstate-mirror etc).
> 
> Should I just change the description and commit message so that it
> doesn't promote it for distro config to use it or should I keep it all
> private?
> 
> I'm thinking about using this also in my bitbake world builds and it's
> also easier to enable it in config there, than parsing the log and
> calling send-error-report from jenkins job config when report was
> created.

I can see the usecase for this, equally, we do need to be mindful of the
privacy concerns. FWIW, I do want to see the class in use on the
yoctoproject autobuilders, however I had envisaged we'd just add a
command after the "bitbake" one which would hoover up any errors and
report them.

I think I'm ok with the idea, as long as it only gets triggered if the
user adds a specific file into HOME. 

I don't like the idea of using raw_input() in the bbclass file, I think
this is fragile and likely to cause issues. How about we have an
external script which handles setup of this file? I'd like to see a big
warning the user has to acknowledge in setting up this such that they
realise that all build errors may get reported.

I'd also like to have two separate classes since I'd like to have the
"saving errors" part enabled by default for everyone. The "sending
errors" part would enough happen automatically if configured, otherwise
the user could run it manually. 

> Should I use error-report-web installed on yoctoproject.org or create my own
> instance somewhere?

I'm hoping we can try and share a server in the hope that we can then
figure out statistics on errors. If 50 people see a given error we can
then prioritise it compared to an error only 2 people see. I know
Michael is working on setting something up.

Cheers,

Richard

Patch

diff --git a/meta/classes/report-error.bbclass b/meta/classes/report-error.bbclass
index 479b38d..58bce0d 100644
--- a/meta/classes/report-error.bbclass
+++ b/meta/classes/report-error.bbclass
@@ -7,6 +7,12 @@ 
 # Licensed under the MIT license, see COPYING.MIT for details
 
 ERR_REPORT_DIR ?= "${LOG_DIR}/error-report"
+ERR_REPORT_PORT ?= "80"
+
+ERR_REPORT_UPLOAD_FAILURES[type] = "boolean"
+ERR_REPORT_UPLOAD_FAILURES ?= "0"
+ERR_REPORT_UPLOAD_ALL[type] = "boolean"
+ERR_REPORT_UPLOAD_ALL ?= "0"
 
 def errorreport_getdata(e):
     logpath = e.data.getVar('ERR_REPORT_DIR', True)
@@ -24,6 +30,92 @@  def errorreport_savedata(e, newdata, file):
         json.dump(newdata, f, indent=4, sort_keys=True)
     return datafile
 
+def errorreport_get_user_info(e):
+    """
+    Read user info from variables or from git config
+    """
+    import subprocess
+    username = e.data.getVar('ERR_REPORT_USERNAME', True)
+    email = e.data.getVar('ERR_REPORT_EMAIL', True)
+    if not username or email:
+        # try to read them from git config
+        username = subprocess.check_output(['git', 'config', '--get', 'user.name']).strip()
+        email = subprocess.check_output(['git', 'config', '--get', 'user.email']).strip()
+    return (username, email)
+
+def errorreport_get_submit_info(e):
+    """
+    Read submit info from ~/.oe-send-error or ask interactively and save it there.
+    """
+    home = os.path.expanduser("~")
+    userfile = os.path.join(home, ".oe-send-error")
+    if os.path.isfile(userfile):
+        with open(userfile) as g:
+            username = g.readline()
+            email = g.readline()
+    else:
+        print("Please enter your name and your email (optionally), they'll be saved in the file you send.")
+        username = raw_input("Name: ")
+        email = raw_input("E-mail (not required): ")
+        server = raw_input("Server: ")
+        port = raw_input("Port: ")
+        if len(username) > 0 and len(username) < 50:
+            with open(userfile, "w") as g:
+                g.write(username + "\n")
+                g.write(email + "\n")
+                g.write(server + "\n")
+                g.write(port + "\n")
+        else:
+            print("Invalid inputs, try again.")
+            return errorreport_get_submit_info()
+        return (username, email, server, port)
+
+def errorreport_senddata(e, json_file):
+    """
+    From scripts/send-error-report to automate report submissions.
+    """
+
+    import httplib, urllib, os, sys, json, subprocess
+
+    if os.path.isfile(json_file):
+        server = e.data.getVar('ERR_REPORT_SERVER', True)
+        port = e.data.getVar('ERR_REPORT_PORT', True)
+        bb.note("Uploading the report to %s:%s" % (server, port))
+
+        with open(json_file) as f:
+            data = f.read()
+
+        try:
+            jsondata = json.loads(data)
+            if not jsondata['username'] or not server:
+                (username, email, server, port) = errorreport_get_submit_info(e)
+                jsondata['username'] = username.strip()
+                jsondata['email'] = email.strip()
+            data = json.dumps(jsondata, indent=4, sort_keys=True)
+        except:
+            bb.error("Invalid json data")
+            return
+
+        try:
+            params = urllib.urlencode({'data': data})
+            headers = {"Content-type": "application/json"}
+            conn = httplib.HTTPConnection(server, port)
+            conn.request("POST", "/ClientPost/", params, headers)
+            response = conn.getresponse()
+            res = response.read()
+            if response.status == 200:
+                bb.note(res)
+            else:
+                bb.warn("There was a problem submiting your data, response written in %s.response.html" % json_file)
+                with open("%s.response.html" % json_file, "w") as f:
+                    f.write(res)
+            conn.close()
+        except:
+            bb.warn("Server connection failed")
+
+    else:
+        bb.warn("No data file found.")
+
 python errorreport_handler () {
         import json
 
@@ -38,6 +130,9 @@  python errorreport_handler () {
             data['failures'] = []
             data['component'] = e.getPkgs()[0]
             data['branch_commit'] = base_detect_branch(e.data) + ": " + base_detect_revision(e.data)
+            (username, email) = errorreport_get_user_info(e)
+            data['username'] = username.strip()
+            data['email'] = email.strip()
             errorreport_savedata(e, data, "error-report.txt")
 
         elif isinstance(e, bb.build.TaskFailed):
@@ -55,11 +150,15 @@  python errorreport_handler () {
 
         elif isinstance(e, bb.event.BuildCompleted):
             jsondata = json.loads(errorreport_getdata(e))
+            upload_failures = oe.data.typed_value('ERR_REPORT_UPLOAD_FAILURES', e.data)
+            upload_all = oe.data.typed_value('ERR_REPORT_UPLOAD_ALL', e.data)
             failures = jsondata['failures']
-            if(len(failures) > 0):
+            if failures or upload_all:
                 filename = "error_report_" + e.data.getVar("BUILDNAME")+".txt"
                 datafile = errorreport_savedata(e, jsondata, filename)
-                bb.note("The errors of this build are stored in: %s. You can send the errors to an upstream server by running: send-error-report %s [server]" % (datafile, datafile))
+                bb.note("The report of this build are stored in: %s." % (datafile))
+                if upload_all or (failures and upload_failures):
+                    errorreport_senddata(e, datafile)
 }
 
 addhandler errorreport_handler
diff --git a/meta/conf/local.conf.sample.extended b/meta/conf/local.conf.sample.extended
index 5f10886..14d39e2 100644
--- a/meta/conf/local.conf.sample.extended
+++ b/meta/conf/local.conf.sample.extended
@@ -304,3 +304,23 @@ 
 #INITRAMFS_IMAGE = "core-image-minimal-initramfs"
 #INITRAMFS_IMAGE_BUNDLE = "1"
 
+#
+# Uploading reports with report-error.bbclass
+#
+# Allow to automatically upload the reports without developer calling extra script to upload them.
+#
+# ERR_REPORT_SERVER = "caprica.palm.com"
+# ERR_REPORT_PORT = "8000"
+#
+# Username if you don't want to use "git config --get user.name"
+# ERR_REPORT_USERNAME = "Tester"
+# E-mail if you don't want to use "git config --get user.email"
+# ERR_REPORT_EMAIL = "tester@test.ltd"
+#
+# Upload reports from failed builds
+# ERR_REPORT_UPLOAD_FAILURES = "1"
+#
+# Upload reports from all builds, not only those with failures
+# ERR_REPORT_UPLOAD_ALL = "1"
+#
+# INHERIT += "report-error"