Skip to content

Commit 33da1e3

Browse files
tomvangoethemWebRTC LUCI CQ
authored andcommitted
[M137] Add metrics for restricted candidates
This adds two UMA metrics for the type of SDP munging that occurred and the port of the candidate that was restricted. The metrics descriptions are being added here: crrev.com/c/6521706. (cherry picked from commit d5b3b1ed4594821451d2ebb1d61cc1e6dab83b56) Bug: b/409713509 Fixed: b/417142969 Change-Id: I3232cb0cee6848074ab103f4d4edb2e080e09568 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/390340 Reviewed-by: Harald Alvestrand <[email protected]> Reviewed-by: Johannes Kron <[email protected]> Commit-Queue: Tom Van Goethem <[email protected]> Cr-Original-Commit-Position: refs/heads/main@{#44559} Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/390920 Reviewed-by: Tomas Gunnarsson <[email protected]> Commit-Queue: Harald Alvestrand <[email protected]> Cr-Commit-Position: refs/branch-heads/7151@{#2} Cr-Branched-From: dc428bd-refs/heads/main@{#44472}
1 parent c8a619a commit 33da1e3

File tree

2 files changed

+28
-0
lines changed

2 files changed

+28
-0
lines changed

pc/sdp_offer_answer.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5335,6 +5335,18 @@ bool SdpOfferAnswerHandler::ReadyToUseRemoteCandidate(
53355335
RTC_LOG(LS_ERROR) << "ReadyToUseRemoteCandidate: Candidate not valid "
53365336
"because of SDP munging.";
53375337
*valid = false;
5338+
// There might be other types of SDP munging, but here we're only
5339+
// interested in IceUfrag and IcePwd.
5340+
SdpMungingType sdp_munging_type =
5341+
last_sdp_munging_type_ == SdpMungingType::kIcePwd
5342+
? SdpMungingType::kIcePwd
5343+
: SdpMungingType::kIceUfrag;
5344+
RTC_HISTOGRAM_ENUMERATION_SPARSE(
5345+
"WebRTC.PeerConnection.RestrictedCandidates.SdpMungingType",
5346+
sdp_munging_type, SdpMungingType::kMaxValue);
5347+
RTC_HISTOGRAM_ENUMERATION_SPARSE(
5348+
"WebRTC.PeerConnection.RestrictedCandidates.Port",
5349+
candidate->candidate().address().port(), 65536);
53385350
return false;
53395351
}
53405352
}

pc/sdp_offer_answer_unittest.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,11 @@
1818
#include <vector>
1919

2020
#include "absl/strings/match.h"
21+
#include "absl/strings/numbers.h"
2122
#include "absl/strings/str_cat.h"
2223
#include "absl/strings/str_replace.h"
24+
#include "absl/strings/str_split.h"
25+
#include "absl/strings/string_view.h"
2326
#include "api/audio_codecs/audio_format.h"
2427
#include "api/audio_codecs/builtin_audio_decoder_factory.h"
2528
#include "api/audio_codecs/builtin_audio_encoder_factory.h"
@@ -1984,6 +1987,7 @@ TEST_F(SdpOfferAnswerMungingTest, IceUfragRestrictedAddresses) {
19841987
{"127.0.1.1:23456", true}, {"8.8.8.8:3456", true},
19851988
};
19861989

1990+
int num_blocked = 0;
19871991
for (const auto& address_test : address_tests) {
19881992
std::optional<RTCError> result;
19891993
const std::string candidate = StringFormat(
@@ -1999,8 +2003,20 @@ TEST_F(SdpOfferAnswerMungingTest, IceUfragRestrictedAddresses) {
19992003
if (address_test.second == true) {
20002004
EXPECT_TRUE(result.value().ok());
20012005
} else {
2006+
std::pair<absl::string_view, absl::string_view> host =
2007+
absl::StrSplit(address_test.first, ":");
2008+
int port;
2009+
ASSERT_TRUE(absl::SimpleAtoi(host.second, &port));
20022010
EXPECT_FALSE(result.value().ok());
20032011
EXPECT_EQ(result.value().type(), RTCErrorType::UNSUPPORTED_OPERATION);
2012+
num_blocked++;
2013+
EXPECT_THAT(
2014+
metrics::Samples(
2015+
"WebRTC.PeerConnection.RestrictedCandidates.SdpMungingType"),
2016+
ElementsAre(Pair(SdpMungingType::kIceUfrag, num_blocked)));
2017+
EXPECT_THAT(
2018+
metrics::Samples("WebRTC.PeerConnection.RestrictedCandidates.Port"),
2019+
Contains(Pair(port, 1)));
20042020
}
20052021
}
20062022
}

0 commit comments

Comments
 (0)