From 014a7bc9b0a234938eac5d4058235da0565ff814 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 25 Sep 2017 08:36:38 -0700 Subject: [PATCH 1/6] Fix bytes downloaded stat tracking --- interface/src/Application.cpp | 14 ++++++++------ .../scripting/AssetMappingsScriptingInterface.cpp | 1 + libraries/networking/src/AssetResourceRequest.cpp | 6 +++--- libraries/networking/src/AssetResourceRequest.h | 2 ++ libraries/networking/src/FileResourceRequest.cpp | 3 +-- libraries/networking/src/HTTPResourceRequest.cpp | 9 ++++----- libraries/networking/src/ResourceRequest.cpp | 11 +++++++++++ libraries/networking/src/ResourceRequest.h | 2 ++ libraries/shared/src/StatTracker.cpp | 4 ++-- libraries/shared/src/StatTracker.h | 6 +++--- 10 files changed, 37 insertions(+), 21 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 480928ccd2..43541c3ac2 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1628,12 +1628,14 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo properties["throttled"] = _displayPlugin ? _displayPlugin->isThrottled() : false; QJsonObject bytesDownloaded; - bytesDownloaded["atp"] = statTracker->getStat(STAT_ATP_RESOURCE_TOTAL_BYTES).toInt(); - bytesDownloaded["http"] = statTracker->getStat(STAT_HTTP_RESOURCE_TOTAL_BYTES).toInt(); - bytesDownloaded["file"] = statTracker->getStat(STAT_FILE_RESOURCE_TOTAL_BYTES).toInt(); - bytesDownloaded["total"] = bytesDownloaded["atp"].toInt() + bytesDownloaded["http"].toInt() - + bytesDownloaded["file"].toInt(); - properties["bytesDownloaded"] = bytesDownloaded; + auto atpBytes = statTracker->getStat(STAT_ATP_RESOURCE_TOTAL_BYTES).toLongLong(); + auto httpBytes = statTracker->getStat(STAT_HTTP_RESOURCE_TOTAL_BYTES).toLongLong(); + auto fileBytes = statTracker->getStat(STAT_FILE_RESOURCE_TOTAL_BYTES).toLongLong(); + bytesDownloaded["atp"] = atpBytes; + bytesDownloaded["http"] = httpBytes; + bytesDownloaded["file"] = fileBytes; + bytesDownloaded["total"] = atpBytes + httpBytes + fileBytes; + properties["bytes_downloaded"] = bytesDownloaded; auto myAvatar = getMyAvatar(); glm::vec3 avatarPosition = myAvatar->getPosition(); diff --git a/interface/src/scripting/AssetMappingsScriptingInterface.cpp b/interface/src/scripting/AssetMappingsScriptingInterface.cpp index 5031016c3f..0912c869d3 100644 --- a/interface/src/scripting/AssetMappingsScriptingInterface.cpp +++ b/interface/src/scripting/AssetMappingsScriptingInterface.cpp @@ -21,6 +21,7 @@ #include #include #include +#include static const int AUTO_REFRESH_INTERVAL = 1000; diff --git a/libraries/networking/src/AssetResourceRequest.cpp b/libraries/networking/src/AssetResourceRequest.cpp index 2dc22a9337..55d0ad4f75 100644 --- a/libraries/networking/src/AssetResourceRequest.cpp +++ b/libraries/networking/src/AssetResourceRequest.cpp @@ -155,6 +155,7 @@ void AssetResourceRequest::requestHash(const AssetHash& hash) { case AssetRequest::Error::NoError: _data = req->getData(); _result = Success; + recordBytesDownloadedInStats(STAT_ATP_RESOURCE_TOTAL_BYTES, _data.size()); break; case AssetRequest::InvalidHash: _result = InvalidURL; @@ -202,9 +203,8 @@ void AssetResourceRequest::onDownloadProgress(qint64 bytesReceived, qint64 bytes emit progress(bytesReceived, bytesTotal); auto now = p_high_resolution_clock::now(); - - // Recording ATP bytes downloaded in stats - DependencyManager::get()->updateStat(STAT_ATP_RESOURCE_TOTAL_BYTES, bytesReceived); + + recordBytesDownloadedInStats(STAT_ATP_RESOURCE_TOTAL_BYTES, bytesReceived); // if we haven't received the full asset check if it is time to output progress to log // we do so every X seconds to assist with ATP download tracking diff --git a/libraries/networking/src/AssetResourceRequest.h b/libraries/networking/src/AssetResourceRequest.h index 18b82f2573..ac36c83985 100644 --- a/libraries/networking/src/AssetResourceRequest.h +++ b/libraries/networking/src/AssetResourceRequest.h @@ -41,6 +41,8 @@ private: AssetRequest* _assetRequest { nullptr }; p_high_resolution_clock::time_point _lastProgressDebug; + + int64_t _lastRecordedBytesDownloaded { 0 }; }; #endif diff --git a/libraries/networking/src/FileResourceRequest.cpp b/libraries/networking/src/FileResourceRequest.cpp index 26857716e1..dfff21dae6 100644 --- a/libraries/networking/src/FileResourceRequest.cpp +++ b/libraries/networking/src/FileResourceRequest.cpp @@ -69,8 +69,7 @@ void FileResourceRequest::doSend() { if (_result == ResourceRequest::Success) { statTracker->incrementStat(STAT_FILE_REQUEST_SUCCESS); - // Recording FILE bytes downloaded in stats - statTracker->updateStat(STAT_FILE_RESOURCE_TOTAL_BYTES,fileSize); + statTracker->updateStat(STAT_FILE_RESOURCE_TOTAL_BYTES, fileSize); } else { statTracker->incrementStat(STAT_FILE_REQUEST_FAILED); } diff --git a/libraries/networking/src/HTTPResourceRequest.cpp b/libraries/networking/src/HTTPResourceRequest.cpp index edba520bd5..a34f92aaa6 100644 --- a/libraries/networking/src/HTTPResourceRequest.cpp +++ b/libraries/networking/src/HTTPResourceRequest.cpp @@ -141,6 +141,8 @@ void HTTPResourceRequest::onRequestFinished() { } } + recordBytesDownloadedInStats(STAT_HTTP_RESOURCE_TOTAL_BYTES, _data.size()); + break; case QNetworkReply::TimeoutError: @@ -201,11 +203,8 @@ void HTTPResourceRequest::onDownloadProgress(qint64 bytesReceived, qint64 bytesT _sendTimer->start(); emit progress(bytesReceived, bytesTotal); - - // Recording HTTP bytes downloaded in stats - DependencyManager::get()->updateStat(STAT_HTTP_RESOURCE_TOTAL_BYTES, bytesReceived); - - + + recordBytesDownloadedInStats(STAT_HTTP_RESOURCE_TOTAL_BYTES, bytesReceived); } void HTTPResourceRequest::onTimeout() { diff --git a/libraries/networking/src/ResourceRequest.cpp b/libraries/networking/src/ResourceRequest.cpp index f028535e2f..5d39583c9e 100644 --- a/libraries/networking/src/ResourceRequest.cpp +++ b/libraries/networking/src/ResourceRequest.cpp @@ -11,6 +11,9 @@ #include "ResourceRequest.h" +#include +#include + #include ResourceRequest::ResourceRequest(const QUrl& url) : _url(url) { } @@ -40,3 +43,11 @@ QString ResourceRequest::getResultString() const { default: return "Unspecified Error"; } } + +void ResourceRequest::recordBytesDownloadedInStats(const QString& statName, int64_t bytesReceived) { + auto dBytes = bytesReceived - _lastRecordedBytesDownloaded; + if (dBytes > 0) { + _lastRecordedBytesDownloaded = bytesReceived; + DependencyManager::get()->updateStat(statName, dBytes); + } +} diff --git a/libraries/networking/src/ResourceRequest.h b/libraries/networking/src/ResourceRequest.h index cf852c1e1b..8dd09ccc49 100644 --- a/libraries/networking/src/ResourceRequest.h +++ b/libraries/networking/src/ResourceRequest.h @@ -85,6 +85,7 @@ signals: protected: virtual void doSend() = 0; + void recordBytesDownloadedInStats(const QString& statName, int64_t bytesReceived); QUrl _url; QUrl _relativePathURL; @@ -97,6 +98,7 @@ protected: ByteRange _byteRange; bool _rangeRequestSuccessful { false }; uint64_t _totalSizeOfResource { 0 }; + int64_t _lastRecordedBytesDownloaded { 0 }; }; #endif diff --git a/libraries/shared/src/StatTracker.cpp b/libraries/shared/src/StatTracker.cpp index 0ac9ab0092..1884037ce8 100644 --- a/libraries/shared/src/StatTracker.cpp +++ b/libraries/shared/src/StatTracker.cpp @@ -17,12 +17,12 @@ QVariant StatTracker::getStat(const QString& name) { return _stats[name]; } -void StatTracker::setStat(const QString& name, int value) { +void StatTracker::setStat(const QString& name, int64_t value) { Lock lock(_statsLock); _stats[name] = value; } -void StatTracker::updateStat(const QString& name, int value) { +void StatTracker::updateStat(const QString& name, int64_t value) { Lock lock(_statsLock); auto itr = _stats.find(name); if (_stats.end() == itr) { diff --git a/libraries/shared/src/StatTracker.h b/libraries/shared/src/StatTracker.h index 38afc2c379..4b84bbcb32 100644 --- a/libraries/shared/src/StatTracker.h +++ b/libraries/shared/src/StatTracker.h @@ -24,15 +24,15 @@ class StatTracker : public Dependency { public: StatTracker(); QVariant getStat(const QString& name); - void setStat(const QString& name, int value); - void updateStat(const QString& name, int mod); + void setStat(const QString& name, int64_t value); + void updateStat(const QString& name, int64_t mod); void incrementStat(const QString& name); void decrementStat(const QString& name); private: using Mutex = std::mutex; using Lock = std::lock_guard; Mutex _statsLock; - QHash _stats; + QHash _stats; }; class CounterStat { From 88810d28c1f39c19ffc1d5f4a29aa0fdcf3d0afd Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Mon, 25 Sep 2017 08:40:14 -0700 Subject: [PATCH 2/6] Add activity logging for uploading_asset --- .../AssetMappingsScriptingInterface.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/interface/src/scripting/AssetMappingsScriptingInterface.cpp b/interface/src/scripting/AssetMappingsScriptingInterface.cpp index 0912c869d3..b9ce273901 100644 --- a/interface/src/scripting/AssetMappingsScriptingInterface.cpp +++ b/interface/src/scripting/AssetMappingsScriptingInterface.cpp @@ -105,6 +105,25 @@ void AssetMappingsScriptingInterface::uploadFile(QString path, QString mapping, startedCallback.call(); + QFile file { path }; + int64_t size { 0 }; + if (file.open(QIODevice::ReadOnly)) { + size = file.size(); + file.close(); + } + + QString extension = ""; + auto idx = path.lastIndexOf("."); + if (idx) { + extension = path.mid(idx + 1); + } + + UserActivityLogger::getInstance().logAction("uploading_asset", { + { "size", size }, + { "mapping", mapping }, + { "extension", extension} + }); + auto upload = DependencyManager::get()->createUpload(path); QObject::connect(upload, &AssetUpload::finished, this, [=](AssetUpload* upload, const QString& hash) mutable { if (upload->getError() != AssetUpload::NoError) { From 0e3c2f2e50ca12a569dd9c1ba619fda44fcb5fd8 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Wed, 27 Sep 2017 15:22:23 -0700 Subject: [PATCH 3/6] Fix ambiguous conversion to QVariant in StatTracker --- libraries/shared/src/StatTracker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/shared/src/StatTracker.cpp b/libraries/shared/src/StatTracker.cpp index 1884037ce8..adfafd0ea7 100644 --- a/libraries/shared/src/StatTracker.cpp +++ b/libraries/shared/src/StatTracker.cpp @@ -14,7 +14,7 @@ StatTracker::StatTracker() { QVariant StatTracker::getStat(const QString& name) { std::lock_guard lock(_statsLock); - return _stats[name]; + return QVariant::fromValue(_stats[name]); } void StatTracker::setStat(const QString& name, int64_t value) { From 97bf093cabda4dfcb98d2c1416b122533ef8898b Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 28 Sep 2017 08:16:54 -0700 Subject: [PATCH 4/6] Fix ambiguous conversion in AssetMappingScriptingInterface --- interface/src/scripting/AssetMappingsScriptingInterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/scripting/AssetMappingsScriptingInterface.cpp b/interface/src/scripting/AssetMappingsScriptingInterface.cpp index b9ce273901..ba533a5bd1 100644 --- a/interface/src/scripting/AssetMappingsScriptingInterface.cpp +++ b/interface/src/scripting/AssetMappingsScriptingInterface.cpp @@ -119,7 +119,7 @@ void AssetMappingsScriptingInterface::uploadFile(QString path, QString mapping, } UserActivityLogger::getInstance().logAction("uploading_asset", { - { "size", size }, + { "size", (qint64)size }, { "mapping", mapping }, { "extension", extension} }); From ff019d619516cb1e9b91006c311ff0bdca7a3730 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Thu, 28 Sep 2017 13:56:49 -0700 Subject: [PATCH 5/6] Fix extension detection when tracking asset uploads --- interface/src/scripting/AssetMappingsScriptingInterface.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/interface/src/scripting/AssetMappingsScriptingInterface.cpp b/interface/src/scripting/AssetMappingsScriptingInterface.cpp index ba533a5bd1..2d288a8804 100644 --- a/interface/src/scripting/AssetMappingsScriptingInterface.cpp +++ b/interface/src/scripting/AssetMappingsScriptingInterface.cpp @@ -114,7 +114,7 @@ void AssetMappingsScriptingInterface::uploadFile(QString path, QString mapping, QString extension = ""; auto idx = path.lastIndexOf("."); - if (idx) { + if (idx >= 0) { extension = path.mid(idx + 1); } @@ -384,4 +384,4 @@ void AssetMappingModel::setupRoles() { roleNames[Qt::DisplayRole] = "name"; roleNames[Qt::UserRole + 5] = "baked"; setItemRoleNames(roleNames); -} \ No newline at end of file +} From 9c120f1194e9994abbfe89257de16466e3485440 Mon Sep 17 00:00:00 2001 From: Ryan Huffman Date: Tue, 10 Oct 2017 09:44:13 -0700 Subject: [PATCH 6/6] Update AssetMappingsScriptingInterface to not open file for activity event --- .../src/scripting/AssetMappingsScriptingInterface.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/interface/src/scripting/AssetMappingsScriptingInterface.cpp b/interface/src/scripting/AssetMappingsScriptingInterface.cpp index 2d288a8804..b8fd8f0f0d 100644 --- a/interface/src/scripting/AssetMappingsScriptingInterface.cpp +++ b/interface/src/scripting/AssetMappingsScriptingInterface.cpp @@ -105,12 +105,8 @@ void AssetMappingsScriptingInterface::uploadFile(QString path, QString mapping, startedCallback.call(); - QFile file { path }; - int64_t size { 0 }; - if (file.open(QIODevice::ReadOnly)) { - size = file.size(); - file.close(); - } + QFileInfo fileInfo { path }; + int64_t size { fileInfo.size() }; QString extension = ""; auto idx = path.lastIndexOf(".");