Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add clearer logging of exchange ids. #9861

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/app/CommandSender.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ CHIP_ERROR CommandSender::OnMessageReceived(Messaging::ExchangeContext * apExcha

void CommandSender::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
{
ChipLogProgress(DataManagement, "Time out! failed to receive invoke command response from Exchange: %d",
apExchangeContext->GetExchangeId());
ChipLogProgress(DataManagement, "Time out! failed to receive invoke command response from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(apExchangeContext));

if (mpDelegate != nullptr)
{
Expand Down
3 changes: 2 additions & 1 deletion src/app/InteractionModelEngine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ CHIP_ERROR InteractionModelEngine::OnMessageReceived(Messaging::ExchangeContext

void InteractionModelEngine::OnResponseTimeout(Messaging::ExchangeContext * ec)
{
ChipLogProgress(InteractionModel, "Time out! failed to receive echo response from Exchange: %d", ec->GetExchangeId());
ChipLogProgress(InteractionModel, "Time out! failed to receive echo response from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(ec));
}

CHIP_ERROR InteractionModelEngine::SendReadRequest(ReadPrepareParams & aReadPrepareParams, uint64_t aAppIdentifier)
Expand Down
4 changes: 2 additions & 2 deletions src/app/ReadClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,8 +449,8 @@ CHIP_ERROR ReadClient::ProcessReportData(System::PacketBufferHandle && aPayload)

void ReadClient::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
{
ChipLogProgress(DataManagement, "Time out! failed to receive report data from Exchange: %d",
apExchangeContext->GetExchangeId());
ChipLogProgress(DataManagement, "Time out! failed to receive report data from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(apExchangeContext));
ShutdownInternal(CHIP_ERROR_TIMEOUT);
}

Expand Down
4 changes: 2 additions & 2 deletions src/app/ReadHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ CHIP_ERROR ReadHandler::OnUnknownMsgType(Messaging::ExchangeContext * apExchange

void ReadHandler::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
{
ChipLogProgress(DataManagement, "Time out! failed to receive status response from Exchange: %d",
apExchangeContext->GetExchangeId());
ChipLogProgress(DataManagement, "Time out! failed to receive status response from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(apExchangeContext));
Shutdown();
}

Expand Down
4 changes: 2 additions & 2 deletions src/app/WriteClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,8 @@ CHIP_ERROR WriteClient::OnMessageReceived(Messaging::ExchangeContext * apExchang

void WriteClient::OnResponseTimeout(Messaging::ExchangeContext * apExchangeContext)
{
ChipLogProgress(DataManagement, "Time out! failed to receive write response from Exchange: %d",
apExchangeContext->GetExchangeId());
ChipLogProgress(DataManagement, "Time out! failed to receive write response from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(apExchangeContext));

if (mpDelegate != nullptr)
{
Expand Down
3 changes: 2 additions & 1 deletion src/controller/CHIPDeviceController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,8 @@ CHIP_ERROR DeviceController::OnMessageReceived(Messaging::ExchangeContext * ec,

void DeviceController::OnResponseTimeout(Messaging::ExchangeContext * ec)
{
ChipLogProgress(Controller, "Time out! failed to receive response from Exchange: %p", ec);
ChipLogProgress(Controller, "Time out! failed to receive response from Exchange: " ChipLogFormatExchange,
ChipLogValueExchange(ec));
}

void DeviceController::OnNewConnection(SessionHandle session, Messaging::ExchangeManager * mgr)
Expand Down
15 changes: 15 additions & 0 deletions src/lib/support/logging/CHIPLogging.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,5 +323,20 @@ bool IsCategoryEnabled(uint8_t category);
*/
#define ChipLogValueMEI(aValue) static_cast<uint16_t>(aValue >> 16), static_cast<uint16_t>(aValue)

/**
* Logging helpers for exchanges. For now just log the exchange id and whether
* it's an initiator or responder, but eventually we may want to log the peer
* node id as well (especially for the responder case). Some callsites only
* have the exchange id and initiator/responder boolean, not an actual exchange,
* so we want to have a helper for that case too.
*/
#define ChipLogFormatExchangeId "%" PRIu16 "%c"
#define ChipLogValueExchangeId(id, isInitiator) id, ((isInitiator) ? 'i' : 'r')
#define ChipLogFormatExchange ChipLogFormatExchangeId
#define ChipLogValueExchange(ec) ChipLogValueExchangeId((ec)->GetExchangeId(), (ec)->IsInitiator())
// A header's initiator boolean is the inverse of the exchange's.
#define ChipLogValueExchangeIdFromHeader(payloadHeader) \
ChipLogValueExchangeId((payloadHeader).GetExchangeID(), !(payloadHeader).IsInitiator())

} // namespace Logging
} // namespace chip
16 changes: 8 additions & 8 deletions src/messaging/ExchangeContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ namespace Messaging {

static void DefaultOnMessageReceived(ExchangeContext * ec, Protocols::Id protocolId, uint8_t msgType, PacketBufferHandle && payload)
{
ChipLogError(ExchangeManager, "Dropping unexpected message %08" PRIX32 ":%d %04" PRIX16, protocolId.ToFullyQualifiedSpecForm(),
msgType, ec->GetExchangeId());
ChipLogError(ExchangeManager, "Dropping unexpected message %08" PRIX32 ":%d " ChipLogFormatExchange,
protocolId.ToFullyQualifiedSpecForm(), msgType, ChipLogValueExchange(ec));
}

bool ExchangeContext::IsInitiator() const
Expand Down Expand Up @@ -191,8 +191,8 @@ void ExchangeContext::Close()
VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() > 0);

#if defined(CHIP_EXCHANGE_CONTEXT_DETAIL_LOGGING)
ChipLogDetail(ExchangeManager, "ec id: %d [%04" PRIX16 "], %s", (this - mExchangeMgr->mContextPool.begin()), mExchangeId,
__func__);
ChipLogDetail(ExchangeManager, "ec id: %d [" ChipLogFormatExchange "], %s", (this - mExchangeMgr->mContextPool.begin()),
ChipLogValueExchange(this), __func__);
#endif

DoClose(false);
Expand All @@ -208,8 +208,8 @@ void ExchangeContext::Abort()
VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() > 0);

#if defined(CHIP_EXCHANGE_CONTEXT_DETAIL_LOGGING)
ChipLogDetail(ExchangeManager, "ec id: %d [%04" PRIX16 "], %s", (this - mExchangeMgr->mContextPool.begin()), mExchangeId,
__func__);
ChipLogDetail(ExchangeManager, "ec id: %d [" ChipLogFormatExchange "], %s", (this - mExchangeMgr->mContextPool.begin()),
ChipLogValueExchange(this), __func__);
#endif

DoClose(true);
Expand Down Expand Up @@ -254,7 +254,7 @@ ExchangeContext::ExchangeContext(ExchangeManager * em, uint16_t ExchangeId, Sess
SetAutoRequestAck(true);

#if defined(CHIP_EXCHANGE_CONTEXT_DETAIL_LOGGING)
ChipLogDetail(ExchangeManager, "ec++ id: %d", ExchangeId);
ChipLogDetail(ExchangeManager, "ec++ id: " ChipLogFormatExchange, ChipLogValueExchange(this));
#endif
SYSTEM_STATS_INCREMENT(chip::System::Stats::kExchangeMgr_NumContexts);
}
Expand Down Expand Up @@ -284,7 +284,7 @@ ExchangeContext::~ExchangeContext()
}

#if defined(CHIP_EXCHANGE_CONTEXT_DETAIL_LOGGING)
ChipLogDetail(ExchangeManager, "ec-- id: %d", mExchangeId);
ChipLogDetail(ExchangeManager, "ec-- id: " ChipLogFormatExchange, ChipLogValueExchange(this));
#endif
SYSTEM_STATS_DECREMENT(chip::System::Stats::kExchangeMgr_NumContexts);
}
Expand Down
8 changes: 5 additions & 3 deletions src/messaging/ExchangeMessageDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,9 @@ CHIP_ERROR ExchangeMessageDispatch::SendMessage(SessionHandle session, uint16_t
#if !defined(NDEBUG)
if (!payloadHeader.HasMessageType(Protocols::SecureChannel::MsgType::StandaloneAck))
{
ChipLogDetail(ExchangeManager, "Piggybacking Ack for MessageCounter:%08" PRIX32 " with msg",
payloadHeader.GetAckMessageCounter().Value());
ChipLogDetail(ExchangeManager,
"Piggybacking Ack for MessageCounter:%08" PRIX32 " on exchange: " ChipLogFormatExchangeId,
payloadHeader.GetAckMessageCounter().Value(), ChipLogValueExchangeId(exchangeId, isInitiator));
}
#endif
}
Expand Down Expand Up @@ -90,7 +91,8 @@ CHIP_ERROR ExchangeMessageDispatch::SendMessage(SessionHandle session, uint16_t
// This is typically a transient situation, so we pretend like this
// packet drop happened somewhere on the network instead of inside
// sendmsg and will just resend it in the normal MRP way later.
ChipLogError(ExchangeManager, "Ignoring ENOBUFS: %" CHIP_ERROR_FORMAT, err.Format());
ChipLogError(ExchangeManager, "Ignoring ENOBUFS: %" CHIP_ERROR_FORMAT " on exchange " ChipLogFormatExchangeId,
err.Format(), ChipLogValueExchangeId(exchangeId, isInitiator));
err = CHIP_NO_ERROR;
}
ReturnErrorOnFailure(err);
Expand Down
11 changes: 7 additions & 4 deletions src/messaging/ExchangeMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,11 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
{
UnsolicitedMessageHandler * matchingUMH = nullptr;

ChipLogProgress(ExchangeManager, "Received message of type 0x%02x with vendorId 0x%04x and protocolId 0x%04x on exchange %d",
payloadHeader.GetMessageType(), payloadHeader.GetProtocolID().GetVendorId(),
payloadHeader.GetProtocolID().GetProtocolId(), payloadHeader.GetExchangeID());
ChipLogProgress(
ExchangeManager,
"Received message of type 0x%02x with vendorId 0x%04x and protocolId 0x%04x on exchange " ChipLogFormatExchangeId,
payloadHeader.GetMessageType(), payloadHeader.GetProtocolID().GetVendorId(), payloadHeader.GetProtocolID().GetProtocolId(),
ChipLogValueExchangeIdFromHeader(payloadHeader));

MessageFlags msgFlags;
if (isDuplicate == DuplicateMessage::Yes)
Expand Down Expand Up @@ -288,7 +290,8 @@ void ExchangeManager::OnMessageReceived(const PacketHeader & packetHeader, const
return;
}

ChipLogDetail(ExchangeManager, "ec id: %d, Delegate: 0x%p", ec->GetExchangeId(), ec->GetDelegate());
ChipLogDetail(ExchangeManager, "Handling via exchange: " ChipLogFormatExchange ", Delegate: 0x%p", ChipLogValueExchange(ec),
ec->GetDelegate());

if (ec->IsEncryptionRequired() != packetHeader.GetFlags().Has(Header::FlagValues::kEncryptedMessage))
{
Expand Down
2 changes: 1 addition & 1 deletion src/protocols/bdx/TransferFacilitator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ CHIP_ERROR TransferFacilitator::OnMessageReceived(chip::Messaging::ExchangeConte

void TransferFacilitator::OnResponseTimeout(Messaging::ExchangeContext * ec)
{
ChipLogError(BDX, "%s, ec: %d", __FUNCTION__, ec->GetExchangeId());
ChipLogError(BDX, "%s, ec: " ChipLogFormatExchange, __FUNCTION__, ChipLogValueExchange(ec));
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
mExchangeCtx = nullptr;
mTransfer.Reset();
}
Expand Down
8 changes: 4 additions & 4 deletions src/transport/SecureSessionMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ CHIP_ERROR SecureSessionMgr::PrepareMessage(SessionHandle session, PayloadHeader

ChipLogProgress(Inet,
"Build %s message %p to 0x" ChipLogFormatX64 " of type %d and protocolId %" PRIu32
" on exchange %d with MessageCounter %" PRIu32 ".",
" on exchange " ChipLogFormatExchangeId " with MessageCounter %" PRIu32 ".",
"encrypted", &preparedMessage, ChipLogValueX64(state->GetPeerNodeId()), payloadHeader.GetMessageType(),
payloadHeader.GetProtocolID().ToFullyQualifiedSpecForm(), payloadHeader.GetExchangeID(),
payloadHeader.GetProtocolID().ToFullyQualifiedSpecForm(), ChipLogValueExchangeIdFromHeader(payloadHeader),
packetHeader.GetMessageCounter());
}
else
Expand All @@ -141,9 +141,9 @@ CHIP_ERROR SecureSessionMgr::PrepareMessage(SessionHandle session, PayloadHeader

ChipLogProgress(Inet,
"Build %s message %p to 0x" ChipLogFormatX64 " of type %d and protocolId %" PRIu32
" on exchange %d with MessageCounter %" PRIu32 ".",
" on exchange " ChipLogFormatExchangeId " with MessageCounter %" PRIu32 ".",
"plaintext", &preparedMessage, ChipLogValueX64(kUndefinedNodeId), payloadHeader.GetMessageType(),
payloadHeader.GetProtocolID().ToFullyQualifiedSpecForm(), payloadHeader.GetExchangeID(),
payloadHeader.GetProtocolID().ToFullyQualifiedSpecForm(), ChipLogValueExchangeIdFromHeader(payloadHeader),
packetHeader.GetMessageCounter());
}

Expand Down