Skip to content

Commit

Permalink
Darwin: add helpers for essential attributes for logging; log unexpec…
Browse files Browse the repository at this point in the history
…ted C-quality attributes (#33560)

* add metric keys for additional metrics

* add metric collection for unexpected C Quality attribute update

* unwind premature optimization placeholders/comments

* rename to indicate the attributes are for logging / informational use

* simplify - pull attributes from cache without intermediate object, should be fast enough

* revert accidental whitespace change

* Restyled by whitespace

* Restyled by clang-format

* make `nil` case possibility more obvious

---------

Co-authored-by: Restyled.io <[email protected]>
  • Loading branch information
2 people authored and pull[bot] committed Jun 18, 2024
1 parent b0cc318 commit 79bcb09
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 2 deletions.
55 changes: 53 additions & 2 deletions src/darwin/Framework/CHIP/MTRDevice.mm
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
#import "MTRError_Internal.h"
#import "MTREventTLVValueDecoder_Internal.h"
#import "MTRLogging_Internal.h"
#import "MTRMetricKeys.h"
#import "MTRMetricsCollector.h"
#import "MTRTimeUtils.h"
#import "MTRUnfairLock.h"
#import "zap-generated/MTRCommandPayloads_Internal.h"
Expand Down Expand Up @@ -1886,9 +1888,16 @@ - (void)_setCachedAttributeValue:(MTRDeviceDataValueDictionary _Nullable)value f
&& isFromSubscription
&& !_receivingPrimingReport
&& AttributeHasChangesOmittedQuality(path)) {
// Do not persist new values for Changes Omitted Quality attributes unless
// they're part of a Priming Report or from a read response.
// Do not persist new values for Changes Omitted Quality (aka C Quality)
// attributes unless they're part of a Priming Report or from a read response.
// (removals are OK)

// log when a device violates expectations for Changes Omitted Quality attributes.
using namespace chip::Tracing::DarwinFramework;
MATTER_LOG_METRIC_BEGIN(kMetricUnexpectedCQualityUpdate);
[self _addInformationalAttributesToCurrentMetricScope];
MATTER_LOG_METRIC_END(kMetricUnexpectedCQualityUpdate);

return;
}

Expand Down Expand Up @@ -3642,6 +3651,48 @@ - (void)removeClientDataForKey:(NSString *)key endpointID:(NSNumber *)endpointID
[self.temporaryMetaDataCache removeObjectForKey:[NSString stringWithFormat:@"%@:%@", key, endpointID]];
}

#pragma mark Log Help

- (nullable NSNumber *)_informationalNumberAtAttributePath:(MTRAttributePath *)attributePath
{
auto * cachedData = [self _cachedAttributeValueForPath:attributePath];

auto * attrReport = [[MTRAttributeReport alloc] initWithResponseValue:@{
MTRAttributePathKey : attributePath,
MTRDataKey : cachedData,
}
error:nil];

return attrReport.value;
}

- (nullable NSNumber *)_informationalVendorID
{
auto * vendorIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId)
clusterID:@(MTRClusterIDTypeBasicInformationID)
attributeID:@(MTRClusterBasicAttributeVendorIDID)];

return [self _informationalNumberAtAttributePath:vendorIDPath];
}

- (nullable NSNumber *)_informationalProductID
{
auto * productIDPath = [MTRAttributePath attributePathWithEndpointID:@(kRootEndpointId)
clusterID:@(MTRClusterIDTypeBasicInformationID)
attributeID:@(MTRClusterBasicAttributeProductIDID)];

return [self _informationalNumberAtAttributePath:productIDPath];
}

- (void)_addInformationalAttributesToCurrentMetricScope
{
using namespace chip::Tracing::DarwinFramework;
MATTER_LOG_METRIC(kMetricDeviceVendorID, [self _informationalVendorID].unsignedShortValue);
MATTER_LOG_METRIC(kMetricDeviceProductID, [self _informationalProductID].unsignedShortValue);
BOOL usesThread = [self _deviceUsesThread];
MATTER_LOG_METRIC(kMetricDeviceUsesThread, usesThread);
}

@end

/* BEGIN DRAGONS: Note methods here cannot be renamed, and are used by private callers, do not rename, remove or modify behavior here */
Expand Down
6 changes: 6 additions & 0 deletions src/darwin/Framework/CHIP/MTRMetricKeys.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ constexpr Tracing::MetricKey kMetricDeviceVendorID = "dwnfw_device_vendor_id";
// Device Product ID
constexpr Tracing::MetricKey kMetricDeviceProductID = "dwnfw_device_product_id";

// Device Uses Thread
constexpr Tracing::MetricKey kMetricDeviceUsesThread = "dwnfw_device_uses_thread_bool";

// Counter of number of devices discovered on the network during setup
constexpr Tracing::MetricKey kMetricOnNetworkDevicesAdded = "dwnfw_onnet_devices_added";

Expand All @@ -81,6 +84,9 @@ constexpr Tracing::MetricKey kMetricBLEDevicesAdded = "dwnfw_ble_devices_added";
// Counter of number of BLE devices removed during setup
constexpr Tracing::MetricKey kMetricBLEDevicesRemoved = "dwnfw_ble_devices_removed";

// Unexpected C quality attribute update outside of priming
constexpr Tracing::MetricKey kMetricUnexpectedCQualityUpdate = "dwnpm_bad_c_attr_update";

} // namespace DarwinFramework
} // namespace Tracing
} // namespace chip

0 comments on commit 79bcb09

Please sign in to comment.