From 2e0c0c08a243fbb4dbe299b5338e2a5583240447 Mon Sep 17 00:00:00 2001 From: Dan Minor Date: Tue, 1 Oct 2019 19:45:02 +0000 Subject: [PATCH] Bug 1571015 - Improve WebRTC Call Duration Telemetry; r=drno,ng This adds a reference counted implementation of Telemetry::AutoTimer for use in timing WebRTC calls. The telemetry is recorded when the reference count goes from one to zero. This allows capturing the total call duration, rather than the duration of individual PeerConnections. The timers are stored in a HashTable, indexed by the Document URI, allowing for call durations to be calculated individually for separate pages. This reuses the existing WEBRTC_CALL_DURATION telemetry key. This means we won't be able to compare telemetry from versions of Firefox with this change to versions of Firefox without this change. The reason for making this change is that the telemetry is not very useful in its current form, so I think it makes sense to do this rather than adding a new key. Differential Revision: https://phabricator.services.mozilla.com/D47800 --HG-- extra : moz-landing-system : lando --- .../src/peerconnection/PeerConnectionImpl.cpp | 51 ++++++++++++++----- .../src/peerconnection/PeerConnectionImpl.h | 25 ++++++--- 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp index 5fc54313f2b8..3c195c2f0d44 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp @@ -229,6 +229,19 @@ RTCStatsQuery::RTCStatsQuery(bool aInternal, bool aRecordTelemetry) RTCStatsQuery::~RTCStatsQuery() {} +void PeerConnectionAutoTimer::AddRef() { mRefCnt++; } + +void PeerConnectionAutoTimer::Release() { + mRefCnt--; + if (mRefCnt == 0) { + Telemetry::Accumulate( + Telemetry::WEBRTC_CALL_DURATION, + static_cast((TimeStamp::Now() - mStart).ToSeconds())); + } +} + +bool PeerConnectionAutoTimer::IsStopped() { return mRefCnt == 0; } + NS_IMPL_ISUPPORTS0(PeerConnectionImpl) already_AddRefed PeerConnectionImpl::Constructor( @@ -2145,6 +2158,20 @@ void PeerConnectionImpl::RecordEndOfCallTelemetry() const { type |= kDataChannelTypeMask; } Telemetry::Accumulate(Telemetry::WEBRTC_CALL_TYPE, type); + + if (mWindow) { + nsCString spec; + nsresult rv = mWindow->GetDocumentURI()->GetSpec(spec); + if (NS_SUCCEEDED(rv)) { + auto itor = mAutoTimers.find(spec.BeginReading()); + if (itor != mAutoTimers.end()) { + itor->second.Release(); + if (itor->second.IsStopped()) { + mAutoTimers.erase(itor); + } + } + } + } } nsresult PeerConnectionImpl::CloseInt() { @@ -2197,13 +2224,6 @@ void PeerConnectionImpl::ShutdownMedia() { } } - // End of call to be recorded in Telemetry - if (!mStartTime.IsNull()) { - TimeDuration timeDelta = TimeStamp::Now() - mStartTime; - Telemetry::Accumulate(Telemetry::WEBRTC_CALL_DURATION, - timeDelta.ToSeconds()); - } - // Forget the reference so that we can transfer it to // SelfDestruct(). mMedia.forget().take()->SelfDestruct(); @@ -2898,13 +2918,19 @@ void PeerConnectionImpl::RecordIceRestartStatistics(JsepSdpType type) { // Telemetry for when calls start void PeerConnectionImpl::startCallTelem() { - if (!mStartTime.IsNull()) { - return; + if (mWindow) { + nsCString spec; + nsresult rv = mWindow->GetDocumentURI()->GetSpec(spec); + if (NS_SUCCEEDED(rv)) { + auto itor = mAutoTimers.find(spec.BeginReading()); + if (itor == mAutoTimers.end()) { + mAutoTimers.emplace(spec.BeginReading(), PeerConnectionAutoTimer()); + } else { + itor->second.AddRef(); + } + } } - // Start time for calls - mStartTime = TimeStamp::Now(); - // Increment session call counter // If we want to track Loop calls independently here, we need two histograms. Telemetry::Accumulate(Telemetry::WEBRTC_CALL_COUNT_2, 1); @@ -2971,4 +2997,5 @@ PeerConnectionImpl::DTMFState::~DTMFState() { StopPlayout(); } NS_IMPL_ISUPPORTS(PeerConnectionImpl::DTMFState, nsITimerCallback) +std::map PeerConnectionImpl::mAutoTimers; } // namespace mozilla diff --git a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h index 0ec67df8f863..4ec408236053 100644 --- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h +++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.h @@ -5,7 +5,6 @@ #ifndef _PEER_CONNECTION_IMPL_H_ #define _PEER_CONNECTION_IMPL_H_ -#include #include #include #include @@ -140,6 +139,20 @@ class RTCStatsQuery { DOMHighResTimeStamp now; }; +// This is a variation of Telemetry::AutoTimer that keeps a reference +// count and records the elapsed time when the count falls to zero. The +// elapsed time is recorded in seconds. +struct PeerConnectionAutoTimer { + PeerConnectionAutoTimer() : mRefCnt(1), mStart(TimeStamp::Now()){}; + void AddRef(); + void Release(); + bool IsStopped(); + + private: + int64_t mRefCnt; + TimeStamp mStart; +}; + typedef MozPromise, nsresult, true> RTCStatsQueryPromise; @@ -622,13 +635,13 @@ class PeerConnectionImpl final unsigned long mIceRestartCount; unsigned long mIceRollbackCount; - // Start time of ICE, used for telemetry + // The following are used for Telemetry: + // Start time of ICE. mozilla::TimeStamp mIceStartTime; - // Start time of call used for Telemetry - mozilla::TimeStamp mStartTime; - // Flag if we have transitioned from checking to connected or failed, used - // for Telemetry + // Flag if we have transitioned from checking to connected or failed. bool mIceFinished = false; + // Hold PeerConnectionAutoTimer instances for each window. + static std::map mAutoTimers; bool mHaveConfiguredCodecs;