Add metrics for SDP munging outcome
The outcome of SDP munging is logged to metrics. This checks whether any
munging was done, and if so, whether it was accepted or rejected. The
metrics are defined in https://crrev.com/c/6520602.
Bug: chromium:40567530
Change-Id: Icf55f7bd089214160b0231b232a191ee3940bf34
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/390401
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Auto-Submit: Tom Van Goethem <tov@google.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44558}
diff --git a/api/uma_metrics.h b/api/uma_metrics.h
index fdca7dc..e6f722c 100644
--- a/api/uma_metrics.h
+++ b/api/uma_metrics.h
@@ -227,6 +227,16 @@
kMaxValue,
};
+// The outcome of setting the local description, whether SDP munging is detected
+// and if the should be accepted or rejected. Keep in sync with
+// SdpMungingOutcome from tools/metrics/histograms/metadata/web_rtc/enums.xml
+enum class SdpMungingOutcome {
+ kNoMunge = 0,
+ kAccepted = 1,
+ kRejected = 2,
+ kMaxValue = kRejected,
+};
+
// When adding new metrics please consider using the style described in
// https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histograms/README.md#usage
// instead of the legacy enums used above.
diff --git a/pc/sdp_munging_detector_unittest.cc b/pc/sdp_munging_detector_unittest.cc
index a06735b..ea2ac1e 100644
--- a/pc/sdp_munging_detector_unittest.cc
+++ b/pc/sdp_munging_detector_unittest.cc
@@ -273,6 +273,12 @@
EXPECT_THAT(
metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
ElementsAre(Pair(SdpMungingType::kIceUfrag, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.SdpOutcome.Rejected"),
+ ElementsAre(Pair(SdpMungingType::kIceUfrag, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.Outcome"),
+ ElementsAre(Pair(static_cast<int>(SdpMungingOutcome::kRejected), 1)));
}
TEST_F(SdpMungingTest, IceUfragCheckDisabledByFieldTrial) {
@@ -291,6 +297,12 @@
EXPECT_THAT(
metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
ElementsAre(Pair(SdpMungingType::kIceUfrag, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.SdpOutcome.Accepted"),
+ ElementsAre(Pair(SdpMungingType::kIceUfrag, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.Outcome"),
+ ElementsAre(Pair(static_cast<int>(SdpMungingOutcome::kAccepted), 1)));
}
TEST_F(SdpMungingTest, IceUfragWithCheckDisabledForTesting) {
@@ -324,6 +336,12 @@
EXPECT_THAT(
metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
ElementsAre(Pair(SdpMungingType::kIcePwd, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.SdpOutcome.Accepted"),
+ ElementsAre(Pair(SdpMungingType::kIcePwd, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.Outcome"),
+ ElementsAre(Pair(static_cast<int>(SdpMungingOutcome::kAccepted), 1)));
}
TEST_F(SdpMungingTest, IcePwd) {
@@ -340,6 +358,12 @@
EXPECT_THAT(
metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
ElementsAre(Pair(SdpMungingType::kIcePwd, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.SdpOutcome.Rejected"),
+ ElementsAre(Pair(SdpMungingType::kIcePwd, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.Outcome"),
+ ElementsAre(Pair(static_cast<int>(SdpMungingOutcome::kRejected), 1)));
}
TEST_F(SdpMungingTest, IceUfragRestrictedAddresses) {
@@ -416,6 +440,12 @@
EXPECT_THAT(
metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
ElementsAre(Pair(SdpMungingType::kIceUfrag, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.SdpOutcome.Rejected"),
+ ElementsAre(Pair(SdpMungingType::kIceUfrag, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.Outcome"),
+ ElementsAre(Pair(static_cast<int>(SdpMungingOutcome::kRejected), 1)));
}
TEST_F(SdpMungingTest, IceMode) {
@@ -500,6 +530,12 @@
EXPECT_THAT(
metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
ElementsAre(Pair(SdpMungingType::kNumberOfContents, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.SdpOutcome.Rejected"),
+ ElementsAre(Pair(SdpMungingType::kNumberOfContents, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.Outcome"),
+ ElementsAre(Pair(static_cast<int>(SdpMungingOutcome::kRejected), 1)));
}
TEST_F(SdpMungingTest, RemoveContentKillswitch) {
@@ -523,6 +559,12 @@
EXPECT_THAT(
metrics::Samples("WebRTC.PeerConnection.SdpMunging.Offer.Initial"),
ElementsAre(Pair(SdpMungingType::kNumberOfContents, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.SdpOutcome.Accepted"),
+ ElementsAre(Pair(SdpMungingType::kNumberOfContents, 1)));
+ EXPECT_THAT(
+ metrics::Samples("WebRTC.PeerConnection.SdpMunging.Outcome"),
+ ElementsAre(Pair(static_cast<int>(SdpMungingOutcome::kAccepted), 1)));
}
TEST_F(SdpMungingTest, TransceiverDirection) {
diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 6f19d1b..ecb4e8b8 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc
@@ -2486,14 +2486,27 @@
"WebRTC-NoSdpMangleNumberOfContents")) {
reject_error = true;
}
+ SdpMungingOutcome outcome = reject_error ? SdpMungingOutcome::kRejected
+ : SdpMungingOutcome::kAccepted;
+ RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.SdpMunging.Outcome",
+ static_cast<int>(outcome),
+ static_cast<int>(SdpMungingOutcome::kMaxValue));
if (reject_error) {
observer->OnSetLocalDescriptionComplete(
RTCError(RTCErrorType::INVALID_MODIFICATION,
"SDP is modified in a non-acceptable way"));
last_sdp_munging_type_ = sdp_munging_type;
ReportInitialSdpMunging(had_local_description, desc->GetType());
+ RTC_HISTOGRAM_ENUMERATION_SPARSE(
+ "WebRTC.PeerConnection.SdpMunging.SdpOutcome.Rejected",
+ sdp_munging_type, SdpMungingType::kMaxValue);
return;
}
+ if (sdp_munging_type != kNoModification) {
+ RTC_HISTOGRAM_ENUMERATION_SPARSE(
+ "WebRTC.PeerConnection.SdpMunging.SdpOutcome.Accepted",
+ sdp_munging_type, SdpMungingType::kMaxValue);
+ }
}
// Workaround for isses.webrtc.org/412904801 - detect if packetization:raw
