Delete UMA histograms relating to Plan B vs Unified Plan.
Plan B having been deleted from Chrome, there is no need to collect UMAs
relating to Plan B vs Unified Plan setups.
Bug: chromium:1357994
Change-Id: Icb5d16823ea9d849798583cd1c82683014b8a15c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275309
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38069}
diff --git a/api/uma_metrics.h b/api/uma_metrics.h
index a975b82..c4fecf0 100644
--- a/api/uma_metrics.h
+++ b/api/uma_metrics.h
@@ -112,36 +112,6 @@
kSdpSemanticRequestMax
};
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
-enum SdpSemanticNegotiated {
- kSdpSemanticNegotiatedNone = 0,
- kSdpSemanticNegotiatedPlanB = 1,
- kSdpSemanticNegotiatedUnifiedPlan = 2,
- kSdpSemanticNegotiatedMixed = 3,
- kSdpSemanticNegotiatedMax
-};
-
-// Metric which records the format of the received SDP for tracking how much the
-// difference between Plan B and Unified Plan affect users.
-// These values are persisted to logs. Entries should not be renumbered and
-// numeric values should never be reused.
-enum SdpFormatReceived {
- // No audio or video tracks. This is worth special casing since it seems to be
- // the most common scenario (data-channel only).
- kSdpFormatReceivedNoTracks = 0,
- // No more than one audio and one video track. Should be compatible with both
- // Plan B and Unified Plan endpoints.
- kSdpFormatReceivedSimple = 1,
- // More than one audio track or more than one video track in the Plan B format
- // (e.g., one audio media section with multiple streams).
- kSdpFormatReceivedComplexPlanB = 2,
- // More than one audio track or more than one video track in the Unified Plan
- // format (e.g., two audio media sections).
- kSdpFormatReceivedComplexUnifiedPlan = 3,
- kSdpFormatReceivedMax
-};
-
// Metric for counting the outcome of adding an ICE candidate
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 517669d..e05a2b7 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -2559,50 +2559,6 @@
return true;
}
-void PeerConnection::ReportSdpFormatReceived(
- const SessionDescriptionInterface& remote_description) {
- int num_audio_mlines = 0;
- int num_video_mlines = 0;
- int num_audio_tracks = 0;
- int num_video_tracks = 0;
- for (const ContentInfo& content :
- remote_description.description()->contents()) {
- cricket::MediaType media_type = content.media_description()->type();
- int num_tracks = std::max(
- 1, static_cast<int>(content.media_description()->streams().size()));
- if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- num_audio_mlines += 1;
- num_audio_tracks += num_tracks;
- } else if (media_type == cricket::MEDIA_TYPE_VIDEO) {
- num_video_mlines += 1;
- num_video_tracks += num_tracks;
- }
- }
- SdpFormatReceived format = kSdpFormatReceivedNoTracks;
- if (num_audio_mlines > 1 || num_video_mlines > 1) {
- format = kSdpFormatReceivedComplexUnifiedPlan;
- } else if (num_audio_tracks > 1 || num_video_tracks > 1) {
- format = kSdpFormatReceivedComplexPlanB;
- } else if (num_audio_tracks > 0 || num_video_tracks > 0) {
- format = kSdpFormatReceivedSimple;
- }
- switch (remote_description.GetType()) {
- case SdpType::kOffer:
- // Historically only offers were counted.
- RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpFormatReceived",
- format, kSdpFormatReceivedMax);
- break;
- case SdpType::kAnswer:
- RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpFormatReceivedAnswer",
- format, kSdpFormatReceivedMax);
- break;
- default:
- RTC_LOG(LS_ERROR) << "Can not report SdpFormatReceived for "
- << SdpTypeToString(remote_description.GetType());
- break;
- }
-}
-
void PeerConnection::ReportSdpBundleUsage(
const SessionDescriptionInterface& remote_description) {
RTC_DCHECK_RUN_ON(signaling_thread());
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 4398058..36a9d2a 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -371,10 +371,6 @@
void AddRemoteCandidate(const std::string& mid,
const cricket::Candidate& candidate) override;
- // Report the UMA metric SdpFormatReceived for the given remote description.
- void ReportSdpFormatReceived(
- const SessionDescriptionInterface& remote_description) override;
-
// Report the UMA metric BundleUsage for the given remote description.
void ReportSdpBundleUsage(
const SessionDescriptionInterface& remote_description) override;
diff --git a/pc/peer_connection_internal.h b/pc/peer_connection_internal.h
index 1bca156..ecf8fbf 100644
--- a/pc/peer_connection_internal.h
+++ b/pc/peer_connection_internal.h
@@ -53,10 +53,6 @@
virtual const PeerConnectionInterface::RTCConfiguration* configuration()
const = 0;
- // Report the UMA metric SdpFormatReceived for the given remote description.
- virtual void ReportSdpFormatReceived(
- const SessionDescriptionInterface& remote_description) = 0;
-
// Report the UMA metric BundleUsage for the given remote description.
virtual void ReportSdpBundleUsage(
const SessionDescriptionInterface& remote_description) = 0;
diff --git a/pc/peer_connection_rtp_unittest.cc b/pc/peer_connection_rtp_unittest.cc
index 268101a..e17c52b 100644
--- a/pc/peer_connection_rtp_unittest.cc
+++ b/pc/peer_connection_rtp_unittest.cc
@@ -1811,10 +1811,6 @@
auto* answer = caller->pc()->remote_description();
EXPECT_EQ(cricket::kMsidSignalingMediaSection,
answer->description()->msid_signaling());
- // Check that this is counted correctly
- EXPECT_METRIC_THAT(
- metrics::Samples("WebRTC.PeerConnection.SdpSemanticNegotiated"),
- ElementsAre(Pair(kSdpSemanticNegotiatedUnifiedPlan, 2)));
}
TEST_F(PeerConnectionMsidSignalingTest, PlanBOfferToUnifiedPlanAnswer) {
@@ -1890,92 +1886,6 @@
answer->description()->msid_signaling());
}
-// Test that the correct UMA metrics are reported for simple/complex SDP.
-
-class SdpFormatReceivedTest : public PeerConnectionRtpTestUnifiedPlan {};
-
-#ifdef WEBRTC_HAVE_SCTP
-TEST_F(SdpFormatReceivedTest, DataChannelOnlyIsReportedAsNoTracks) {
- auto caller = CreatePeerConnectionWithUnifiedPlan();
- caller->CreateDataChannel("dc");
- auto callee = CreatePeerConnectionWithUnifiedPlan();
-
- ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- // Note that only the callee does ReportSdpFormatReceived.
- EXPECT_METRIC_THAT(
- metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
- ElementsAre(Pair(kSdpFormatReceivedNoTracks, 1)));
-}
-#endif // WEBRTC_HAVE_SCTP
-
-TEST_F(SdpFormatReceivedTest, SimpleUnifiedPlanIsReportedAsSimple) {
- auto caller = CreatePeerConnectionWithUnifiedPlan();
- caller->AddAudioTrack("audio");
- caller->AddVideoTrack("video");
- auto callee = CreatePeerConnectionWithPlanB();
-
- ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- // Note that only the callee does ReportSdpFormatReceived.
- EXPECT_METRIC_THAT(
- metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
- ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
-}
-
-TEST_F(SdpFormatReceivedTest, SimplePlanBIsReportedAsSimple) {
- auto caller = CreatePeerConnectionWithPlanB();
- caller->AddVideoTrack("video"); // Video only.
- auto callee = CreatePeerConnectionWithUnifiedPlan();
-
- ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- // Note that only the callee does ReportSdpFormatReceived.
- EXPECT_METRIC_THAT(
- metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
- ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
-}
-
-TEST_F(SdpFormatReceivedTest, ComplexUnifiedIsReportedAsComplexUnifiedPlan) {
- auto caller = CreatePeerConnectionWithUnifiedPlan();
- caller->AddAudioTrack("audio1");
- caller->AddAudioTrack("audio2");
- caller->AddVideoTrack("video");
- auto callee = CreatePeerConnectionWithPlanB();
-
- ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOffer()));
- // Note that only the callee does ReportSdpFormatReceived.
- EXPECT_METRIC_THAT(
- metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
- ElementsAre(Pair(kSdpFormatReceivedComplexUnifiedPlan, 1)));
-}
-
-TEST_F(SdpFormatReceivedTest, ComplexPlanBIsReportedAsComplexPlanB) {
- auto caller = CreatePeerConnectionWithPlanB();
- caller->AddVideoTrack("video1");
- caller->AddVideoTrack("video2");
- auto callee = CreatePeerConnectionWithUnifiedPlan();
-
- // This fails since Unified Plan cannot set a session description with
- // multiple "Plan B tracks" in the same media section. But we still expect the
- // SDP Format to be recorded.
- ASSERT_FALSE(callee->SetRemoteDescription(caller->CreateOffer()));
- // Note that only the callee does ReportSdpFormatReceived.
- EXPECT_METRIC_THAT(
- metrics::Samples("WebRTC.PeerConnection.SdpFormatReceived"),
- ElementsAre(Pair(kSdpFormatReceivedComplexPlanB, 1)));
-}
-
-TEST_F(SdpFormatReceivedTest, AnswerIsReported) {
- auto caller = CreatePeerConnectionWithPlanB();
- caller->AddAudioTrack("audio");
- caller->AddVideoTrack("video");
- auto callee = CreatePeerConnectionWithUnifiedPlan();
-
- ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
- ASSERT_TRUE(caller->SetRemoteDescription(callee->CreateAnswer()));
- EXPECT_METRIC_THAT(
- metrics::Samples("WebRTC.PeerConnection.SdpFormatReceivedAnswer"),
- ElementsAre(Pair(kSdpFormatReceivedSimple, 1)));
-}
-
// Sender setups in a call.
TEST_P(PeerConnectionRtpTest, CreateTwoSendersWithSameTrack) {
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 35e65e0..7b41fce 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -799,7 +799,6 @@
void ReportOfferAnswerUma() {
RTC_DCHECK(ok());
if (type_ == SdpType::kOffer || type_ == SdpType::kAnswer) {
- handler_->pc_->ReportSdpFormatReceived(*desc_.get());
handler_->pc_->ReportSdpBundleUsage(*desc_.get());
}
}
@@ -2200,8 +2199,6 @@
// MaybeStartGathering...
context_->network_thread()->BlockingCall(
[this] { port_allocator()->DiscardCandidatePool(); });
- // Make UMA notes about what was agreed to.
- ReportNegotiatedSdpSemantics(*local_description());
}
observer->OnSetLocalDescriptionComplete(RTCError::OK());
@@ -2404,8 +2401,6 @@
// MaybeStartGathering...
context_->network_thread()->BlockingCall(
[this] { port_allocator()->DiscardCandidatePool(); });
- // Make UMA notes about what was agreed to.
- ReportNegotiatedSdpSemantics(*remote_description());
}
pc_->NoteUsageEvent(UsageEvent::SET_REMOTE_DESCRIPTION_SUCCEEDED);
@@ -4698,30 +4693,6 @@
}
}
-void SdpOfferAnswerHandler::ReportNegotiatedSdpSemantics(
- const SessionDescriptionInterface& answer) {
- SdpSemanticNegotiated semantics_negotiated;
- switch (answer.description()->msid_signaling()) {
- case 0:
- semantics_negotiated = kSdpSemanticNegotiatedNone;
- break;
- case cricket::kMsidSignalingMediaSection:
- semantics_negotiated = kSdpSemanticNegotiatedUnifiedPlan;
- break;
- case cricket::kMsidSignalingSsrcAttribute:
- semantics_negotiated = kSdpSemanticNegotiatedPlanB;
- break;
- case cricket::kMsidSignalingMediaSection |
- cricket::kMsidSignalingSsrcAttribute:
- semantics_negotiated = kSdpSemanticNegotiatedMixed;
- break;
- default:
- RTC_DCHECK_NOTREACHED();
- }
- RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpSemanticNegotiated",
- semantics_negotiated, kSdpSemanticNegotiatedMax);
-}
-
void SdpOfferAnswerHandler::UpdateEndedRemoteMediaStreams() {
RTC_DCHECK_RUN_ON(signaling_thread());
std::vector<rtc::scoped_refptr<MediaStreamInterface>> streams_to_remove;
diff --git a/pc/sdp_offer_answer.h b/pc/sdp_offer_answer.h
index bc5087c..c493dc0 100644
--- a/pc/sdp_offer_answer.h
+++ b/pc/sdp_offer_answer.h
@@ -491,10 +491,6 @@
// `desc` can be null. This means that all channels are deleted.
void RemoveUnusedChannels(const cricket::SessionDescription* desc);
- // Report inferred negotiated SDP semantics from a local/remote answer to the
- // UMA observer.
- void ReportNegotiatedSdpSemantics(const SessionDescriptionInterface& answer);
-
// Finds remote MediaStreams without any tracks and removes them from
// `remote_streams_` and notifies the observer that the MediaStreams no longer
// exist.
diff --git a/pc/test/fake_peer_connection_base.h b/pc/test/fake_peer_connection_base.h
index 9ee69be..e277058 100644
--- a/pc/test/fake_peer_connection_base.h
+++ b/pc/test/fake_peer_connection_base.h
@@ -294,9 +294,6 @@
return nullptr;
}
- void ReportSdpFormatReceived(
- const SessionDescriptionInterface& remote_description) override {}
-
void ReportSdpBundleUsage(
const SessionDescriptionInterface& remote_description) override {}
diff --git a/pc/test/mock_peer_connection_internal.h b/pc/test/mock_peer_connection_internal.h
index 09b3d43..733ec41 100644
--- a/pc/test/mock_peer_connection_internal.h
+++ b/pc/test/mock_peer_connection_internal.h
@@ -203,10 +203,6 @@
(),
(const, override));
MOCK_METHOD(void,
- ReportSdpFormatReceived,
- (const SessionDescriptionInterface&),
- (override));
- MOCK_METHOD(void,
ReportSdpBundleUsage,
(const SessionDescriptionInterface&),
(override));
