{"action":"created","comment":{"url":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/comments/210960357","pull_request_review_id":147275825,"id":210960357,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDIxMDk2MDM1Nw==","diff_hunk":"@@ -21,27 +21,103 @@\n \n #include <fmt/format.h>\n \n-#include <QByteArray>\n #include <QDateTime>\n #include <QEventLoop>\n-#include <QJsonArray>\n #include <QJsonDocument>\n #include <QJsonObject>\n #include <QNetworkReply>\n #include <QNetworkRequest>\n #include <QRegExp>\n-#include <QString>\n \n namespace mp = multipass;\n namespace mpl = multipass::logging;\n \n+using namespace std::literals::chrono_literals;\n+\n namespace\n {\n constexpr auto category = \"metrics\";\n+\n+void post_request(const QUrl& metrics_url, const QByteArray& body)\n+{\n+    QNetworkAccessManager manager;\n+    QNetworkRequest request{metrics_url};\n+    request.setHeader(QNetworkRequest::ContentTypeHeader, \"application/json\");\n+    request.setHeader(QNetworkRequest::ContentLengthHeader, body.size());\n+    request.setHeader(QNetworkRequest::UserAgentHeader, \"multipassd/1.0\");\n+\n+    QEventLoop event_loop;\n+    std::unique_ptr<QNetworkReply> reply;\n+\n+    // For unit testing\n+    if (metrics_url.isLocalFile())\n+        reply = std::unique_ptr<QNetworkReply>(manager.put(request, body));\n+    else\n+        reply = std::unique_ptr<QNetworkReply>(manager.post(request, body));\n+\n+    QObject::connect(reply.get(), &QNetworkReply::finished, &event_loop, &QEventLoop::quit);\n+\n+    event_loop.exec();\n+\n+    auto buff = reply->readAll();\n+\n+    if (reply->error() == QNetworkReply::ProtocolInvalidOperationError)\n+    {\n+        auto error_msg = QJsonDocument::fromJson(buff).object();\n+        mpl::log(mpl::Level::error, category,\n+                 fmt::format(\"Metrics error: {} - {}\", error_msg[\"code\"].toString().toStdString(),\n+                             error_msg[\"message\"].toString().toStdString()));\n+    }\n+    else if (reply->error() != QNetworkReply::NoError)\n+    {\n+        QObject::disconnect(reply.get(), 0, 0, 0);\n+        throw std::runtime_error(fmt::format(\"Metrics error: {}\", reply->errorString().toStdString()));\n+    }\n+}\n }\n \n-mp::MetricsProvider::MetricsProvider(const QUrl& metrics_url, const QString& unique_id)\n-    : metrics_url{metrics_url}, unique_id{unique_id}\n+mp::MetricsProvider::MetricsProvider(const QUrl& url, const QString& unique_id)\n+    : metrics_url{url}, unique_id{unique_id}, metrics_sender{[this] {\n+          std::unique_lock<std::mutex> lck(metrics_mutex, std::defer_lock);\n+          while (true)\n+          {\n+              while (!lck.try_lock())\n+                  std::this_thread::sleep_for(1ms);\n+\n+              if (metric_batches.empty())\n+              {\n+                  metrics_cv.wait(lck);\n+              }\n+\n+              // Null element signals thread to exit\n+              if (metric_batches.first().isNull())\n+              {\n+                  lck.unlock();\n+                  return;\n+              }\n+\n+              auto body = QJsonDocument(metric_batches).toJson(QJsonDocument::Compact);\n+              lck.unlock();\n+\n+              try\n+              {\n+                  post_request(metrics_url, body);\n+                  lck.lock();\n+                  if (metric_batches.first().isNull())\n+                  {\n+                      lck.unlock();\n+                      return;\n+                  }\n+                  metric_batches = QJsonArray{};\n+                  lck.unlock();\n+              }\n+              catch (const std::exception& e)\n+              {\n+                  mpl::log(mpl::Level::error, category, fmt::format(\"{} - Attempting to resend\", e.what()));\n+                  std::this_thread::sleep_for(60s);","path":"src/metrics/metrics_provider.cpp","position":102,"original_position":102,"commit_id":"555e8399390f5f8c698fd8b02c31fa759749d2c8","original_commit_id":"555e8399390f5f8c698fd8b02c31fa759749d2c8","user":{"login":"albaguirre","id":6685297,"node_id":"MDQ6VXNlcjY2ODUyOTc=","avatar_url":"https://avatars2.githubusercontent.com/u/6685297?v=4","gravatar_id":"","url":"https://api.github.com/users/albaguirre","html_url":"https://github.com/albaguirre","followers_url":"https://api.github.com/users/albaguirre/followers","following_url":"https://api.github.com/users/albaguirre/following{/other_user}","gists_url":"https://api.github.com/users/albaguirre/gists{/gist_id}","starred_url":"https://api.github.com/users/albaguirre/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/albaguirre/subscriptions","organizations_url":"https://api.github.com/users/albaguirre/orgs","repos_url":"https://api.github.com/users/albaguirre/repos","events_url":"https://api.github.com/users/albaguirre/events{/privacy}","received_events_url":"https://api.github.com/users/albaguirre/received_events","type":"User","site_admin":false},"body":"I think this should probably be an exponentially backoff timeout.\r\n\r\nAlso instead of sleep (which cannot be aborted) maybe using the timeout version of the condition variable is best.\r\n\r\nSo maybe the loop looks something like:\r\n```\r\nstd::unique_lock<std::mutex> lock{metrics_mutex};\r\nauto timeout = std::chrono::seconds::max(); //max because we don't want to initially time out until metrics are available\r\nwhile (running)\r\n{\r\ncv.wait_for(lock, timeout, [&] { return metrics_available || !running;})\r\n\r\nif (!running) return;\r\nif (!metrics_available) continue; //timed out and no metrics available yet\r\n\r\n<attempt to send metrics>\r\nif (failed)\r\n   timeout = <increase timeout exponentially until some ceiling value?>;\r\nelse\r\n  return;\r\n}\r\n```","created_at":"2018-08-17T16:13:39Z","updated_at":"2018-08-17T16:29:08Z","html_url":"https://github.com/CanonicalLtd/multipass/pull/341#discussion_r210960357","pull_request_url":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341","author_association":"COLLABORATOR","_links":{"self":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/comments/210960357"},"html":{"href":"https://github.com/CanonicalLtd/multipass/pull/341#discussion_r210960357"},"pull_request":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341"}}},"pull_request":{"url":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341","id":208909175,"node_id":"MDExOlB1bGxSZXF1ZXN0MjA4OTA5MTc1","html_url":"https://github.com/CanonicalLtd/multipass/pull/341","diff_url":"https://github.com/CanonicalLtd/multipass/pull/341.diff","patch_url":"https://github.com/CanonicalLtd/multipass/pull/341.patch","issue_url":"https://api.github.com/repos/CanonicalLtd/multipass/issues/341","number":341,"state":"open","locked":false,"title":"metrics_provider: Retry network errors","user":{"login":"townsend2010","id":12715814,"node_id":"MDQ6VXNlcjEyNzE1ODE0","avatar_url":"https://avatars2.githubusercontent.com/u/12715814?v=4","gravatar_id":"","url":"https://api.github.com/users/townsend2010","html_url":"https://github.com/townsend2010","followers_url":"https://api.github.com/users/townsend2010/followers","following_url":"https://api.github.com/users/townsend2010/following{/other_user}","gists_url":"https://api.github.com/users/townsend2010/gists{/gist_id}","starred_url":"https://api.github.com/users/townsend2010/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/townsend2010/subscriptions","organizations_url":"https://api.github.com/users/townsend2010/orgs","repos_url":"https://api.github.com/users/townsend2010/repos","events_url":"https://api.github.com/users/townsend2010/events{/privacy}","received_events_url":"https://api.github.com/users/townsend2010/received_events","type":"User","site_admin":false},"body":"Use a separate thread for the actual sending of the metrics.","created_at":"2018-08-16T16:03:20Z","updated_at":"2018-08-17T16:29:08Z","closed_at":null,"merged_at":null,"merge_commit_sha":"319bcd95fea5cd4965d67cc7397cf3d0fbfbd42a","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341/commits","review_comments_url":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341/comments","review_comment_url":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/comments{/number}","comments_url":"https://api.github.com/repos/CanonicalLtd/multipass/issues/341/comments","statuses_url":"https://api.github.com/repos/CanonicalLtd/multipass/statuses/555e8399390f5f8c698fd8b02c31fa759749d2c8","head":{"label":"townsend2010:metrics-handle-network-failures","ref":"metrics-handle-network-failures","sha":"555e8399390f5f8c698fd8b02c31fa759749d2c8","user":{"login":"townsend2010","id":12715814,"node_id":"MDQ6VXNlcjEyNzE1ODE0","avatar_url":"https://avatars2.githubusercontent.com/u/12715814?v=4","gravatar_id":"","url":"https://api.github.com/users/townsend2010","html_url":"https://github.com/townsend2010","followers_url":"https://api.github.com/users/townsend2010/followers","following_url":"https://api.github.com/users/townsend2010/following{/other_user}","gists_url":"https://api.github.com/users/townsend2010/gists{/gist_id}","starred_url":"https://api.github.com/users/townsend2010/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/townsend2010/subscriptions","organizations_url":"https://api.github.com/users/townsend2010/orgs","repos_url":"https://api.github.com/users/townsend2010/repos","events_url":"https://api.github.com/users/townsend2010/events{/privacy}","received_events_url":"https://api.github.com/users/townsend2010/received_events","type":"User","site_admin":false},"repo":{"id":120343768,"node_id":"MDEwOlJlcG9zaXRvcnkxMjAzNDM3Njg=","name":"multipass","full_name":"townsend2010/multipass","owner":{"login":"townsend2010","id":12715814,"node_id":"MDQ6VXNlcjEyNzE1ODE0","avatar_url":"https://avatars2.githubusercontent.com/u/12715814?v=4","gravatar_id":"","url":"https://api.github.com/users/townsend2010","html_url":"https://github.com/townsend2010","followers_url":"https://api.github.com/users/townsend2010/followers","following_url":"https://api.github.com/users/townsend2010/following{/other_user}","gists_url":"https://api.github.com/users/townsend2010/gists{/gist_id}","starred_url":"https://api.github.com/users/townsend2010/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/townsend2010/subscriptions","organizations_url":"https://api.github.com/users/townsend2010/orgs","repos_url":"https://api.github.com/users/townsend2010/repos","events_url":"https://api.github.com/users/townsend2010/events{/privacy}","received_events_url":"https://api.github.com/users/townsend2010/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/townsend2010/multipass","description":"Multipass orchestrates virtual Ubuntu instances","fork":true,"url":"https://api.github.com/repos/townsend2010/multipass","forks_url":"https://api.github.com/repos/townsend2010/multipass/forks","keys_url":"https://api.github.com/repos/townsend2010/multipass/keys{/key_id}","collaborators_url":"https://api.github.com/repos/townsend2010/multipass/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/townsend2010/multipass/teams","hooks_url":"https://api.github.com/repos/townsend2010/multipass/hooks","issue_events_url":"https://api.github.com/repos/townsend2010/multipass/issues/events{/number}","events_url":"https://api.github.com/repos/townsend2010/multipass/events","assignees_url":"https://api.github.com/repos/townsend2010/multipass/assignees{/user}","branches_url":"https://api.github.com/repos/townsend2010/multipass/branches{/branch}","tags_url":"https://api.github.com/repos/townsend2010/multipass/tags","blobs_url":"https://api.github.com/repos/townsend2010/multipass/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/townsend2010/multipass/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/townsend2010/multipass/git/refs{/sha}","trees_url":"https://api.github.com/repos/townsend2010/multipass/git/trees{/sha}","statuses_url":"https://api.github.com/repos/townsend2010/multipass/statuses/{sha}","languages_url":"https://api.github.com/repos/townsend2010/multipass/languages","stargazers_url":"https://api.github.com/repos/townsend2010/multipass/stargazers","contributors_url":"https://api.github.com/repos/townsend2010/multipass/contributors","subscribers_url":"https://api.github.com/repos/townsend2010/multipass/subscribers","subscription_url":"https://api.github.com/repos/townsend2010/multipass/subscription","commits_url":"https://api.github.com/repos/townsend2010/multipass/commits{/sha}","git_commits_url":"https://api.github.com/repos/townsend2010/multipass/git/commits{/sha}","comments_url":"https://api.github.com/repos/townsend2010/multipass/comments{/number}","issue_comment_url":"https://api.github.com/repos/townsend2010/multipass/issues/comments{/number}","contents_url":"https://api.github.com/repos/townsend2010/multipass/contents/{+path}","compare_url":"https://api.github.com/repos/townsend2010/multipass/compare/{base}...{head}","merges_url":"https://api.github.com/repos/townsend2010/multipass/merges","archive_url":"https://api.github.com/repos/townsend2010/multipass/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/townsend2010/multipass/downloads","issues_url":"https://api.github.com/repos/townsend2010/multipass/issues{/number}","pulls_url":"https://api.github.com/repos/townsend2010/multipass/pulls{/number}","milestones_url":"https://api.github.com/repos/townsend2010/multipass/milestones{/number}","notifications_url":"https://api.github.com/repos/townsend2010/multipass/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/townsend2010/multipass/labels{/name}","releases_url":"https://api.github.com/repos/townsend2010/multipass/releases{/id}","deployments_url":"https://api.github.com/repos/townsend2010/multipass/deployments","created_at":"2018-02-05T18:23:18Z","updated_at":"2018-08-14T18:20:05Z","pushed_at":"2018-08-17T15:02:57Z","git_url":"git://github.com/townsend2010/multipass.git","ssh_url":"git@github.com:townsend2010/multipass.git","clone_url":"https://github.com/townsend2010/multipass.git","svn_url":"https://github.com/townsend2010/multipass","homepage":"","size":772,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":null,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"CanonicalLtd:master","ref":"master","sha":"0a2399e4a8fa65fda7079aa2fc4fecd8954a7071","user":{"login":"CanonicalLtd","id":7490288,"node_id":"MDEyOk9yZ2FuaXphdGlvbjc0OTAyODg=","avatar_url":"https://avatars0.githubusercontent.com/u/7490288?v=4","gravatar_id":"","url":"https://api.github.com/users/CanonicalLtd","html_url":"https://github.com/CanonicalLtd","followers_url":"https://api.github.com/users/CanonicalLtd/followers","following_url":"https://api.github.com/users/CanonicalLtd/following{/other_user}","gists_url":"https://api.github.com/users/CanonicalLtd/gists{/gist_id}","starred_url":"https://api.github.com/users/CanonicalLtd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CanonicalLtd/subscriptions","organizations_url":"https://api.github.com/users/CanonicalLtd/orgs","repos_url":"https://api.github.com/users/CanonicalLtd/repos","events_url":"https://api.github.com/users/CanonicalLtd/events{/privacy}","received_events_url":"https://api.github.com/users/CanonicalLtd/received_events","type":"Organization","site_admin":false},"repo":{"id":114128199,"node_id":"MDEwOlJlcG9zaXRvcnkxMTQxMjgxOTk=","name":"multipass","full_name":"CanonicalLtd/multipass","owner":{"login":"CanonicalLtd","id":7490288,"node_id":"MDEyOk9yZ2FuaXphdGlvbjc0OTAyODg=","avatar_url":"https://avatars0.githubusercontent.com/u/7490288?v=4","gravatar_id":"","url":"https://api.github.com/users/CanonicalLtd","html_url":"https://github.com/CanonicalLtd","followers_url":"https://api.github.com/users/CanonicalLtd/followers","following_url":"https://api.github.com/users/CanonicalLtd/following{/other_user}","gists_url":"https://api.github.com/users/CanonicalLtd/gists{/gist_id}","starred_url":"https://api.github.com/users/CanonicalLtd/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/CanonicalLtd/subscriptions","organizations_url":"https://api.github.com/users/CanonicalLtd/orgs","repos_url":"https://api.github.com/users/CanonicalLtd/repos","events_url":"https://api.github.com/users/CanonicalLtd/events{/privacy}","received_events_url":"https://api.github.com/users/CanonicalLtd/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/CanonicalLtd/multipass","description":"Multipass orchestrates virtual Ubuntu instances","fork":false,"url":"https://api.github.com/repos/CanonicalLtd/multipass","forks_url":"https://api.github.com/repos/CanonicalLtd/multipass/forks","keys_url":"https://api.github.com/repos/CanonicalLtd/multipass/keys{/key_id}","collaborators_url":"https://api.github.com/repos/CanonicalLtd/multipass/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/CanonicalLtd/multipass/teams","hooks_url":"https://api.github.com/repos/CanonicalLtd/multipass/hooks","issue_events_url":"https://api.github.com/repos/CanonicalLtd/multipass/issues/events{/number}","events_url":"https://api.github.com/repos/CanonicalLtd/multipass/events","assignees_url":"https://api.github.com/repos/CanonicalLtd/multipass/assignees{/user}","branches_url":"https://api.github.com/repos/CanonicalLtd/multipass/branches{/branch}","tags_url":"https://api.github.com/repos/CanonicalLtd/multipass/tags","blobs_url":"https://api.github.com/repos/CanonicalLtd/multipass/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/CanonicalLtd/multipass/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/CanonicalLtd/multipass/git/refs{/sha}","trees_url":"https://api.github.com/repos/CanonicalLtd/multipass/git/trees{/sha}","statuses_url":"https://api.github.com/repos/CanonicalLtd/multipass/statuses/{sha}","languages_url":"https://api.github.com/repos/CanonicalLtd/multipass/languages","stargazers_url":"https://api.github.com/repos/CanonicalLtd/multipass/stargazers","contributors_url":"https://api.github.com/repos/CanonicalLtd/multipass/contributors","subscribers_url":"https://api.github.com/repos/CanonicalLtd/multipass/subscribers","subscription_url":"https://api.github.com/repos/CanonicalLtd/multipass/subscription","commits_url":"https://api.github.com/repos/CanonicalLtd/multipass/commits{/sha}","git_commits_url":"https://api.github.com/repos/CanonicalLtd/multipass/git/commits{/sha}","comments_url":"https://api.github.com/repos/CanonicalLtd/multipass/comments{/number}","issue_comment_url":"https://api.github.com/repos/CanonicalLtd/multipass/issues/comments{/number}","contents_url":"https://api.github.com/repos/CanonicalLtd/multipass/contents/{+path}","compare_url":"https://api.github.com/repos/CanonicalLtd/multipass/compare/{base}...{head}","merges_url":"https://api.github.com/repos/CanonicalLtd/multipass/merges","archive_url":"https://api.github.com/repos/CanonicalLtd/multipass/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/CanonicalLtd/multipass/downloads","issues_url":"https://api.github.com/repos/CanonicalLtd/multipass/issues{/number}","pulls_url":"https://api.github.com/repos/CanonicalLtd/multipass/pulls{/number}","milestones_url":"https://api.github.com/repos/CanonicalLtd/multipass/milestones{/number}","notifications_url":"https://api.github.com/repos/CanonicalLtd/multipass/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/CanonicalLtd/multipass/labels{/name}","releases_url":"https://api.github.com/repos/CanonicalLtd/multipass/releases{/id}","deployments_url":"https://api.github.com/repos/CanonicalLtd/multipass/deployments","created_at":"2017-12-13T14:05:21Z","updated_at":"2018-08-14T16:05:22Z","pushed_at":"2018-08-17T15:03:46Z","git_url":"git://github.com/CanonicalLtd/multipass.git","ssh_url":"git@github.com:CanonicalLtd/multipass.git","clone_url":"https://github.com/CanonicalLtd/multipass.git","svn_url":"https://github.com/CanonicalLtd/multipass","homepage":"","size":773,"stargazers_count":46,"watchers_count":46,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":9,"mirror_url":null,"archived":false,"open_issues_count":47,"license":null,"forks":9,"open_issues":47,"watchers":46,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341"},"html":{"href":"https://github.com/CanonicalLtd/multipass/pull/341"},"issue":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/issues/341"},"comments":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/issues/341/comments"},"review_comments":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341/comments"},"review_comment":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/pulls/341/commits"},"statuses":{"href":"https://api.github.com/repos/CanonicalLtd/multipass/statuses/555e8399390f5f8c698fd8b02c31fa759749d2c8"}},"author_association":"COLLABORATOR"}}
{"actor":{"display_login":"albaguirre"}}