mDNS: Add more detailed logging
This CL adds more detailed logging to help debug ongoing issues.
bug: b/157683753
Change-Id: Ibee54a379c95773571cd3e7ec0095aa5f4ac9152
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2223828
Commit-Queue: Ryan Keane <rwkeane@google.com>
Reviewed-by: Jordan Bayles <jophba@chromium.org>
diff --git a/discovery/mdns/mdns_querier.cc b/discovery/mdns/mdns_querier.cc
index 800aef4..7152be2 100644
--- a/discovery/mdns/mdns_querier.cc
+++ b/discovery/mdns/mdns_querier.cc
@@ -372,9 +372,7 @@
for (const MdnsRecord& record : message.answers()) {
if (ShouldAnswerRecordBeProcessed(record)) {
ProcessRecord(record);
- OSP_DVLOG << "\tProcessing answer record for domain '"
- << record.name().ToString() << "' of type '"
- << record.dns_type() << "'...";
+ OSP_DVLOG << "\tProcessing answer record (" << record.ToString() << ")";
found_relevant_records = true;
processed_count++;
}
@@ -385,9 +383,8 @@
// individual records relevant to this querier to update the cache.
for (const MdnsRecord& record : message.additional_records()) {
if (found_relevant_records || ShouldAnswerRecordBeProcessed(record)) {
- OSP_DVLOG << "\tProcessing additional record for domain '"
- << record.name().ToString() << "' of type '"
- << record.dns_type() << "'...";
+ OSP_DVLOG << "\tProcessing additional record (" << record.ToString()
+ << ")";
ProcessRecord(record);
processed_count++;
}
diff --git a/discovery/mdns/mdns_receiver.cc b/discovery/mdns/mdns_receiver.cc
index b21d748..bb8634d 100644
--- a/discovery/mdns/mdns_receiver.cc
+++ b/discovery/mdns/mdns_receiver.cc
@@ -4,6 +4,8 @@
#include "discovery/mdns/mdns_receiver.h"
+#include <utility>
+
#include "discovery/mdns/mdns_reader.h"
#include "util/trace_logging.h"
@@ -66,6 +68,7 @@
MdnsReader reader(config_, packet.data(), packet.size());
MdnsMessage message;
if (!reader.Read(&message)) {
+ OSP_DVLOG << "mDNS message failed to parse...";
return;
}
@@ -74,13 +77,14 @@
client->OnMessageReceived(message);
}
if (response_clients_.empty()) {
- OSP_DVLOG << "Response message dropped. No response client registered...";
+ OSP_DVLOG
+ << "mDNS response message dropped. No response client registered...";
}
} else {
if (query_callback_) {
query_callback_(message, packet.source());
} else {
- OSP_DVLOG << "Query message dropped. No query client registered...";
+ OSP_DVLOG << "mDNS query message dropped. No query client registered...";
}
}
}
diff --git a/discovery/mdns/mdns_records.cc b/discovery/mdns/mdns_records.cc
index a04c269..8be7fe0 100644
--- a/discovery/mdns/mdns_records.cc
+++ b/discovery/mdns/mdns_records.cc
@@ -5,6 +5,9 @@
#include "discovery/mdns/mdns_records.h"
#include <cctype>
+#include <limits>
+#include <sstream>
+#include <vector>
#include "absl/strings/ascii.h"
#include "absl/strings/match.h"
@@ -562,6 +565,34 @@
return name_.MaxWireSize() + absl::visit(wire_size_visitor, rdata_) + 8;
}
+std::string MdnsRecord::ToString() const {
+ std::stringstream ss;
+ ss << "name: '" << name_.ToString() << "'";
+ ss << ", type: " << dns_type_;
+
+ if (dns_type_ == DnsType::kPTR) {
+ const DomainName& target = absl::get<PtrRecordRdata>(rdata_).ptr_domain();
+ ss << ", target: '" << target.ToString() << "'";
+ } else if (dns_type_ == DnsType::kSRV) {
+ const DomainName& target = absl::get<SrvRecordRdata>(rdata_).target();
+ ss << ", target: '" << target.ToString() << "'";
+ } else if (dns_type_ == DnsType::kNSEC) {
+ const auto& nsec_rdata = absl::get<NsecRecordRdata>(rdata_);
+ std::vector<DnsType> types = nsec_rdata.types();
+ ss << ", representing [";
+ if (!types.empty()) {
+ auto it = types.begin();
+ ss << *it++;
+ while (it != types.end()) {
+ ss << ", " << *it++;
+ }
+ ss << "]";
+ }
+ }
+
+ return ss.str();
+}
+
MdnsRecord CreateAddressRecord(DomainName name, const IPAddress& address) {
Rdata rdata;
DnsType type;
diff --git a/discovery/mdns/mdns_records.h b/discovery/mdns/mdns_records.h
index 8e9ac24..696eaf8 100644
--- a/discovery/mdns/mdns_records.h
+++ b/discovery/mdns/mdns_records.h
@@ -419,6 +419,8 @@
record.ttl_.count(), record.rdata_);
}
+ std::string ToString() const;
+
private:
static bool IsValidConfig(const DomainName& name,
DnsType dns_type,
diff --git a/discovery/mdns/mdns_responder.cc b/discovery/mdns/mdns_responder.cc
index 3627f2f..faef9a1 100644
--- a/discovery/mdns/mdns_responder.cc
+++ b/discovery/mdns/mdns_responder.cc
@@ -513,7 +513,7 @@
for (const auto& question : questions) {
OSP_DVLOG << "\tProcessing mDNS Query for domain: '"
<< question.name().ToString() << "', type: '"
- << question.dns_type() << "'";
+ << question.dns_type() << "' from '" << src << "'";
// NSEC records should not be queried for.
if (question.dns_type() == DnsType::kNSEC) {
@@ -586,18 +586,25 @@
// method is called. Exclusive ownership cannot be gained for a record which
// has previously been published, and if this host is the exclusive owner
// then this method will have been called without any delay on the task
- // runner
+ // runner.
ApplyQueryResults(&message, record_handler_, question.name(), known_answers,
question.dns_type(), question.dns_class(),
is_exclusive_owner);
}
// Send the response only if it contains answers to the query.
+ OSP_DVLOG << "\tCompleted Processing mDNS Query for domain: '"
+ << question.name().ToString() << "', type: '" << question.dns_type()
+ << "', with " << message.answers().size() << " results:";
+ for (const auto& record : message.answers()) {
+ OSP_DVLOG << "\t\tanswer (" << record.ToString() << ")";
+ }
+ for (const auto& record : message.additional_records()) {
+ OSP_DVLOG << "\t\tadditional record ('" << record.ToString() << ")";
+ }
+
if (!message.answers().empty()) {
- OSP_DVLOG << "\tmDNS Query processed and response sent!";
send_response(message);
- } else {
- OSP_DVLOG << "\tmDNS Query processed and no response sent!";
}
}
diff --git a/platform/BUILD.gn b/platform/BUILD.gn
index 375ba41..f510fbf 100644
--- a/platform/BUILD.gn
+++ b/platform/BUILD.gn
@@ -202,6 +202,7 @@
"base/error_unittest.cc",
"base/ip_address_unittest.cc",
"base/location_unittest.cc",
+ "base/udp_packet_unittest.cc",
]
# The socket integration tests assume that you can Bind with UDP sockets,
diff --git a/platform/base/udp_packet.cc b/platform/base/udp_packet.cc
index 6cbb0da..8470893 100644
--- a/platform/base/udp_packet.cc
+++ b/platform/base/udp_packet.cc
@@ -1,10 +1,11 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file
+// found in the LICENSE file.
#include "platform/base/udp_packet.h"
#include <cassert>
+#include <sstream>
namespace openscreen {
@@ -26,4 +27,17 @@
UdpPacket& UdpPacket::operator=(UdpPacket&& other) = default;
+std::string UdpPacket::ToString() const {
+ // TODO(issuetracker.google.com/158660166): Change to use shared hex-to-string
+ // method.
+ static constexpr char hex[] = "0123456789ABCDEF";
+ std::stringstream ss;
+ ss << "[";
+ for (auto it = begin(); it != end(); it++) {
+ ss << hex[*it / 16] << hex[*it % 16] << " ";
+ }
+ ss << "]";
+ return ss.str();
+}
+
} // namespace openscreen
diff --git a/platform/base/udp_packet.h b/platform/base/udp_packet.h
index a8fcce0..3a3b262 100644
--- a/platform/base/udp_packet.h
+++ b/platform/base/udp_packet.h
@@ -1,12 +1,13 @@
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file
+// found in the LICENSE file.
#ifndef PLATFORM_BASE_UDP_PACKET_H_
#define PLATFORM_BASE_UDP_PACKET_H_
#include <stdint.h>
+#include <string>
#include <utility>
#include <vector>
@@ -44,6 +45,8 @@
UdpSocket* socket() const { return socket_; }
void set_socket(UdpSocket* socket) { socket_ = socket; }
+ std::string ToString() const;
+
static constexpr size_type kUdpMaxPacketSize = 1 << 16;
private:
diff --git a/platform/base/udp_packet_unittest.cc b/platform/base/udp_packet_unittest.cc
new file mode 100644
index 0000000..8494957
--- /dev/null
+++ b/platform/base/udp_packet_unittest.cc
@@ -0,0 +1,34 @@
+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "platform/base/udp_packet.h"
+
+#include <vector>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace openscreen {
+
+TEST(UdpPacketTest, ValidateToStringNormalCase) {
+ UdpPacket packet{0x73, 0xC7, 0x00, 0x14, 0xFF, 0x2C};
+ std::string result = packet.ToString();
+ EXPECT_EQ(result, "[73 C7 00 14 FF 2C ]");
+
+ UdpPacket packet2{0x1, 0x2, 0x3, 0x4, 0x5};
+ result = packet2.ToString();
+ EXPECT_EQ(result, "[01 02 03 04 05 ]");
+
+ UdpPacket packet3{0x0, 0x0, 0x0};
+ result = packet3.ToString();
+ EXPECT_EQ(result, "[00 00 00 ]");
+}
+
+TEST(UdpPacketTest, ValidateToStringEmpty) {
+ UdpPacket packet{};
+ std::string result = packet.ToString();
+ EXPECT_EQ(result, "[]");
+}
+
+} // namespace openscreen
