Skip to content

Commit

Permalink
Improve discovery logging on Darwin. (#24846)
Browse files Browse the repository at this point in the history 
1) Use progress, not detail, logging, because detail logging is not actually
   persisted in system logs.
2) Add logging to a few functions that were missing it.
  • Loading branch information
@bzbarsky-apple @pull
bzbarsky-apple authored and pull[bot] committed Oct 31, 2024
1 parent 0545066 commit 1224530
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 32 deletions.
16 changes: 8 additions & 8 deletions src/platform/Darwin/DnssdContexts.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ void ResolveContext::DispatchSuccess()
continue;
}

ChipLogDetail(Discovery, "Mdns: Resolve success on interface %" PRIu32, interface.first);
ChipLogProgress(Discovery, "Mdns: Resolve success on interface %" PRIu32, interface.first);
callback(context, &interface.second.service, Span<Inet::IPAddress>(ips.data(), ips.size()), CHIP_NO_ERROR);
break;
}
Expand Down Expand Up @@ -412,11 +412,11 @@ CHIP_ERROR ResolveContext::OnNewAddress(uint32_t interfaceId, const struct socka
ReturnErrorOnFailure(chip::Inet::IPAddress::GetIPAddressFromSockAddr(*address, ip));
interfaces[interfaceId].addresses.push_back(ip);

#ifdef CHIP_DETAIL_LOGGING
#ifdef CHIP_PROGRESS_LOGGING
char addrStr[INET6_ADDRSTRLEN];
ip.ToString(addrStr, sizeof(addrStr));
ChipLogDetail(Discovery, "Mdns: %s interface: %" PRIu32 " ip:%s", __func__, interfaceId, addrStr);
#endif // CHIP_DETAIL_LOGGING
ChipLogProgress(Discovery, "Mdns: %s interface: %" PRIu32 " ip:%s", __func__, interfaceId, addrStr);
#endif // CHIP_PROGRESS_LOGGING

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -449,7 +449,7 @@ bool ResolveContext::HasAddress()
void ResolveContext::OnNewInterface(uint32_t interfaceId, const char * fullname, const char * hostnameWithDomain, uint16_t port,
uint16_t txtLen, const unsigned char * txtRecord)
{
#if CHIP_DETAIL_LOGGING
#if CHIP_PROGRESS_LOGGING
std::string txtString;
auto txtRecordIter = txtRecord;
size_t remainingLen = txtLen;
Expand Down Expand Up @@ -480,9 +480,9 @@ void ResolveContext::OnNewInterface(uint32_t interfaceId, const char * fullname,
txtRecordIter += len;
remainingLen -= len;
}
#endif // CHIP_DETAIL_LOGGING
ChipLogDetail(Discovery, "Mdns : %s hostname:%s fullname:%s interface: %" PRIu32 " port: %u TXT:\"%s\"", __func__,
hostnameWithDomain, fullname, interfaceId, ntohs(port), txtString.c_str());
#endif // CHIP_PROGRESS_LOGGING
ChipLogProgress(Discovery, "Mdns : %s hostname:%s fullname:%s interface: %" PRIu32 " port: %u TXT:\"%s\"", __func__,
hostnameWithDomain, fullname, interfaceId, ntohs(port), txtString.c_str());

InterfaceInfo interface;
interface.service.mPort = ntohs(port);
Expand Down
28 changes: 14 additions & 14 deletions src/platform/Darwin/DnssdHostNameRegistrar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ namespace Dnssd {

namespace {

#if CHIP_DETAIL_LOGGING
#if CHIP_PROGRESS_LOGGING
constexpr const char * kPathStatusInvalid = "Invalid";
constexpr const char * kPathStatusUnsatisfied = "Unsatisfied";
constexpr const char * kPathStatusSatisfied = "Satisfied";
Expand Down Expand Up @@ -118,7 +118,7 @@ void LogDetails(uint32_t interfaceId, InetInterfacesVector inetInterfaces, Inet6
{
char addr[INET_ADDRSTRLEN] = {};
inet_ntop(AF_INET, &inetInterface.second, addr, sizeof(addr));
ChipLogDetail(Discovery, "\t\t* ipv4: %s", addr);
ChipLogProgress(Discovery, "\t\t* ipv4: %s", addr);
}
}

Expand All @@ -128,15 +128,15 @@ void LogDetails(uint32_t interfaceId, InetInterfacesVector inetInterfaces, Inet6
{
char addr[INET6_ADDRSTRLEN] = {};
inet_ntop(AF_INET6, &inet6Interface.second, addr, sizeof(addr));
ChipLogDetail(Discovery, "\t\t* ipv6: %s", addr);
ChipLogProgress(Discovery, "\t\t* ipv6: %s", addr);
}
}
}

void LogDetails(nw_path_t path)
{
auto status = nw_path_get_status(path);
ChipLogDetail(Discovery, "Status: %s", GetPathStatusString(status));
ChipLogProgress(Discovery, "Status: %s", GetPathStatusString(status));
}

void LogDetails(InetInterfacesVector inetInterfaces, Inet6InterfacesVector inet6Interfaces)
Expand All @@ -156,7 +156,7 @@ void LogDetails(InetInterfacesVector inetInterfaces, Inet6InterfacesVector inet6
{
char interfaceName[IFNAMSIZ] = {};
if_indextoname(interfaceId, interfaceName);
ChipLogDetail(Discovery, "\t%s (%u)", interfaceName, interfaceId);
ChipLogProgress(Discovery, "\t%s (%u)", interfaceName, interfaceId);
LogDetails(interfaceId, inetInterfaces, inet6Interfaces);
}
}
Expand All @@ -166,10 +166,10 @@ void LogDetails(nw_interface_t interface, InetInterfacesVector inetInterfaces, I
auto interfaceId = nw_interface_get_index(interface);
auto interfaceName = nw_interface_get_name(interface);
auto interfaceType = nw_interface_get_type(interface);
ChipLogDetail(Discovery, "\t%s (%u / %s)", interfaceName, interfaceId, GetInterfaceTypeString(interfaceType));
ChipLogProgress(Discovery, "\t%s (%u / %s)", interfaceName, interfaceId, GetInterfaceTypeString(interfaceType));
LogDetails(interfaceId, inetInterfaces, inet6Interfaces);
}
#endif // CHIP_DETAIL_LOGGING
#endif // CHIP_PROGRESS_LOGGING

bool HasValidFlags(unsigned int flags, bool allowLoopbackOnly)
{
Expand Down Expand Up @@ -205,7 +205,7 @@ void ShouldUseVersion(chip::Inet::IPAddressType addressType, bool & shouldUseIPv
static void OnRegisterRecord(DNSServiceRef sdRef, DNSRecordRef recordRef, DNSServiceFlags flags, DNSServiceErrorType err,
void * context)
{
ChipLogDetail(Discovery, "Mdns: %s flags: %d", __func__, flags);
ChipLogProgress(Discovery, "Mdns: %s flags: %d", __func__, flags);
if (kDNSServiceErr_NoError != err)
{
ChipLogError(Discovery, "%s (%s)", __func__, Error::ToString(err));
Expand Down Expand Up @@ -304,9 +304,9 @@ CHIP_ERROR HostNameRegistrar::StartMonitorInterfaces(OnInterfaceChanges interfac
nw_path_monitor_set_queue(mInterfaceMonitor, chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue());

nw_path_monitor_set_update_handler(mInterfaceMonitor, ^(nw_path_t path) {
#if CHIP_DETAIL_LOGGING
#if CHIP_PROGRESS_LOGGING
LogDetails(path);
#endif // CHIP_DETAIL_LOGGING
#endif // CHIP_PROGRESS_LOGGING

__block InetInterfacesVector inet;
__block Inet6InterfacesVector inet6;
Expand All @@ -315,9 +315,9 @@ CHIP_ERROR HostNameRegistrar::StartMonitorInterfaces(OnInterfaceChanges interfac
// loopback interface with the specified interface id. If the specified interface id is kDNSServiceInterfaceIndexAny, it
// will look for all available loopback interfaces.
GetInterfaceAddresses(mInterfaceId, mAddressType, inet, inet6, true /* searchLoopbackOnly */);
#if CHIP_DETAIL_LOGGING
#if CHIP_PROGRESS_LOGGING
LogDetails(inet, inet6);
#endif // CHIP_DETAIL_LOGGING
#endif // CHIP_PROGRESS_LOGGING

auto status = nw_path_get_status(path);
if (status == nw_path_status_satisfied)
Expand All @@ -328,9 +328,9 @@ CHIP_ERROR HostNameRegistrar::StartMonitorInterfaces(OnInterfaceChanges interfac

auto targetInterfaceId = nw_interface_get_index(interface);
GetInterfaceAddresses(targetInterfaceId, mAddressType, inet, inet6);
#if CHIP_DETAIL_LOGGING
#if CHIP_PROGRESS_LOGGING
LogDetails(interface, inet, inet6);
#endif // CHIP_DETAIL_LOGGING
#endif // CHIP_PROGRESS_LOGGING
return true;
});
}
Expand Down
25 changes: 15 additions & 10 deletions src/platform/Darwin/DnssdImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ namespace {
static void OnRegister(DNSServiceRef sdRef, DNSServiceFlags flags, DNSServiceErrorType err, const char * name, const char * type,
const char * domain, void * context)
{
ChipLogDetail(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, flags: %d", __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), flags);
ChipLogProgress(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, flags: %d", __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), flags);

auto sdCtx = reinterpret_cast<RegisterContext *>(context);
sdCtx->Finalize(err);
Expand All @@ -181,8 +181,8 @@ static void OnRegister(DNSServiceRef sdRef, DNSServiceFlags flags, DNSServiceErr
CHIP_ERROR Register(void * context, DnssdPublishCallback callback, uint32_t interfaceId, const char * type, const char * name,
uint16_t port, ScopedTXTRecord & record, Inet::IPAddressType addressType, const char * hostname)
{
ChipLogDetail(Discovery, "Registering service %s on host %s with port %u and type: %s on interface id: %" PRIu32,
StringOrNullMarker(name), StringOrNullMarker(hostname), port, StringOrNullMarker(type), interfaceId);
ChipLogProgress(Discovery, "Registering service %s on host %s with port %u and type: %s on interface id: %" PRIu32,
StringOrNullMarker(name), StringOrNullMarker(hostname), port, StringOrNullMarker(type), interfaceId);

RegisterContext * sdCtx = nullptr;
if (CHIP_NO_ERROR == MdnsContexts::GetInstance().GetRegisterContextOfType(type, &sdCtx))
Expand All @@ -207,8 +207,8 @@ CHIP_ERROR Register(void * context, DnssdPublishCallback callback, uint32_t inte

void OnBrowseAdd(BrowseContext * context, const char * name, const char * type, const char * domain, uint32_t interfaceId)
{
ChipLogDetail(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, interface: %" PRIu32, __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);
ChipLogProgress(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, interface: %" PRIu32, __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);

VerifyOrReturn(strcmp(kLocalDot, domain) == 0);

Expand All @@ -234,8 +234,8 @@ void OnBrowseAdd(BrowseContext * context, const char * name, const char * type,

void OnBrowseRemove(BrowseContext * context, const char * name, const char * type, const char * domain, uint32_t interfaceId)
{
ChipLogDetail(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, interface: %" PRIu32, __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);
ChipLogProgress(Discovery, "Mdns: %s name: %s, type: %s, domain: %s, interface: %" PRIu32, __func__, StringOrNullMarker(name),
StringOrNullMarker(type), StringOrNullMarker(domain), interfaceId);

VerifyOrReturn(name != nullptr);
VerifyOrReturn(strcmp(kLocalDot, domain) == 0);
Expand Down Expand Up @@ -282,6 +282,8 @@ CHIP_ERROR Browse(void * context, DnssdBrowseCallback callback, uint32_t interfa
static void OnGetAddrInfo(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t interfaceId, DNSServiceErrorType err,
const char * hostname, const struct sockaddr * address, uint32_t ttl, void * context)
{
ChipLogProgress(Discovery, "Mdns: %s flags: %d, interface: %u, hostname: %s", __func__, flags, (unsigned) interfaceId,
StringOrNullMarker(hostname));
auto sdCtx = reinterpret_cast<ResolveContext *>(context);
ReturnOnFailure(MdnsContexts::GetInstance().Has(sdCtx));
LogOnFailure(__func__, err);
Expand Down Expand Up @@ -316,6 +318,8 @@ static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t inter
const char * fullname, const char * hostname, uint16_t port, uint16_t txtLen, const unsigned char * txtRecord,
void * context)
{
ChipLogProgress(Discovery, "Mdns: %s flags: %d, interface: %u, fullname: %s, hostname: %s, port: %u", __func__, flags,
(unsigned) interfaceId, StringOrNullMarker(fullname), StringOrNullMarker(hostname), port);
auto sdCtx = reinterpret_cast<ResolveContext *>(context);
ReturnOnFailure(MdnsContexts::GetInstance().Has(sdCtx));
LogOnFailure(__func__, err);
Expand All @@ -341,8 +345,8 @@ static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t inter
static CHIP_ERROR Resolve(void * context, DnssdResolveCallback callback, uint32_t interfaceId,
chip::Inet::IPAddressType addressType, const char * type, const char * name)
{
ChipLogDetail(Discovery, "Resolve type=%s name=%s interface=%" PRIu32, StringOrNullMarker(type), StringOrNullMarker(name),
interfaceId);
ChipLogProgress(Discovery, "Resolve type=%s name=%s interface=%" PRIu32, StringOrNullMarker(type), StringOrNullMarker(name),
interfaceId);

// This is a little silly, in that resolves for the same name, type, etc get
// coalesced by the underlying mDNSResponder anyway. But we need to keep
Expand Down Expand Up @@ -489,6 +493,7 @@ CHIP_ERROR ChipDnssdResolve(DnssdService * service, chip::Inet::InterfaceId inte

void ChipDnssdResolveNoLongerNeeded(const char * instanceName)
{
ChipLogProgress(Discovery, "No longer need resolve for %s", instanceName);
auto existingCtx = MdnsContexts::GetInstance().GetExistingResolveForInstanceName(instanceName);
VerifyOrReturn(existingCtx != nullptr);
VerifyOrReturn(*existingCtx->consumerCounter != 0);
Expand Down

0 comments on commit 1224530

Please sign in to comment.