Skip to content

Commit

Permalink
Framework for emitting metrics data (project-chip#32223)
Browse files Browse the repository at this point in the history
* Rerouted tracing macros to Darwin signposts

* Initial framework for logging scalar data event

* Handled the new metrics event changes in collector

* Modified VerifyOrExit macro to accept an optional metric key as third argument

* Removed direct use of chrono in metrics_event.h
Switched MTRMetrics to vend dictionary for metric keys

* Modified SuccessOrExit to optionally accept metric key

* Moved metric keys to separate header
Reworked metric_event names for more clarity

* Switched MATTER_TRACE_METRIC usage to MATTER_LOG_METRIC

* Restyle fixes

* Fixed unit tests

* Fixed build failure due to MetricEvent hidden inside tracing enabled

* Fixing one source of build error

* Fixing darwin build failure

* Code Review: Rename LogMetric to LogMetricEvent

* Code Review Suggestions:

1. Metric Macros take full string constants and no longer use
   preprocessor to prefix. Allows free flowing strings
2. Reworked MetricEvent class and documented
3. Handled LogEventMetric for Darwin, ESP32, Perfetto, JSON to account
   for all types
4. Removed timePoint from MetricEvent class. Timestamps and duration
   calculation is now responsibility for the handlers of the event
5. Reverted BUILD.gn in system to not break out SystemClock.h

* Code Review Feedback #2:

1. Added SuccessOrExitWithMetric and VerifyOrExitWithMetric
2. Cleaned up support .gn to remove dependedency on metrics

* Code Review Feedback #3:

1. Added ScopedMetricEvent to use RAII to track begin and end within a
   scope

* Code Review #4:

Reverted an accidental removal

* Added MTRMetricData to description as per review comment

* Restyler fixes

* Sample code of how Begin and End log metrics can be used

* Fixed compilation error when tracing is disabled

* Fixes for build failures when tracing is disabled

* Picked up code review suggestion accidently dropped

* Code Review Feedback:

1. Begin metric does not take value
2. Allow undefined value for metric
3. Misc other feedback

* Handle undefined value and error value

* Revert a comment change

* Review Feedback: Changed ScopedMetricEvent to capture error by reference

* Fixed another build failure

* Reverting usage of LOG_METRICS

* Review feedback: Fix incorrect documentation

* Code Review Feedback: Remove access to Value in MetricEvent to avoid incorrect access

* Restyler fixes

* Unregistering backend in Darwin shutdown

* Resytler fixes...
  • Loading branch information
anush-apple committed Feb 25, 2024
1 parent c5a0720 commit 8f2a4d2
Show file tree
Hide file tree
Showing 37 changed files with 1,167 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <lib/core/Optional.h>
#include <platform/DiagnosticDataProvider.h>
#include <tracing/macros.h>
#include <tracing/metric_event.h>

using namespace chip;
using namespace chip::app;
Expand Down Expand Up @@ -163,7 +164,7 @@ CHIP_ERROR WiFiDiagosticsAttrAccess::ReadWiFiRssi(AttributeValueEncoder & aEncod
{
rssi.SetNonNull(value);
ChipLogProgress(Zcl, "The current RSSI of the Node’s Wi-Fi radio in dB: %d", value);
MATTER_TRACE_METRIC("wifi_rssi", value);
MATTER_LOG_METRIC(chip::Tracing::kMetricWiFiRSSI, value);
}
else
{
Expand Down
14 changes: 5 additions & 9 deletions src/darwin/Framework/CHIP/MTRDeviceControllerDelegateBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@

#import "MTRDeviceControllerDelegateBridge.h"
#import "MTRDeviceController.h"
#import "MTRDeviceController_Internal.h"
#import "MTRError_Internal.h"
#import "MTRLogging_Internal.h"
#import "MTRMetrics_Internal.h"
#import "MTRMetricsCollector.h"

MTRDeviceControllerDelegateBridge::MTRDeviceControllerDelegateBridge(void)
: mDelegate(nil)
Expand Down Expand Up @@ -130,15 +131,10 @@
nodeID = @(nodeId);
}

// If the client implements the metrics delegate, prefer that over others
if ([strongDelegate respondsToSelector:@selector(controller:commissioningComplete:nodeID:metrics:)]) {
MTRMetrics * metrics = [MTRMetrics new];

if (nsError) {
[metrics setValue:nsError forKey:MTRMetricCommissioningStatusKey];
} else {
auto * error = [NSError errorWithDomain:MTRErrorDomain code:0 userInfo:nil];
[metrics setValue:error forKey:MTRMetricCommissioningStatusKey];
}
// Create a snapshot and clear for next operation
MTRMetrics * metrics = [[MTRMetricsCollector sharedInstance] metricSnapshot:TRUE];
[strongDelegate controller:strongController commissioningComplete:nsError nodeID:nodeID metrics:metrics];
} else {
[strongDelegate controller:strongController commissioningComplete:nsError nodeID:nodeID];
Expand Down
7 changes: 7 additions & 0 deletions src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#import "MTRFabricInfo_Internal.h"
#import "MTRFramework.h"
#import "MTRLogging_Internal.h"
#import "MTRMetricsCollector.h"
#import "MTROTAProviderDelegateBridge.h"
#import "MTROperationalBrowser.h"
#import "MTRP256KeypairBridge.h"
Expand Down Expand Up @@ -349,6 +350,8 @@ - (void)cleanupStartupObjects
}

_diagnosticLogsDownloader = nil;

ShutdownMetricsCollection();
}

- (CHIP_ERROR)_initFabricTable:(FabricTable &)fabricTable
Expand Down Expand Up @@ -414,6 +417,10 @@ - (BOOL)startControllerFactory:(MTRDeviceControllerFactoryParams *)startupParams
return YES;
}

// Register any tracing backends. This has to be done before starting the event loop to run registering
// the tracing backend in the right queue context
StartupMetricsCollection();

DeviceLayer::PlatformMgrImpl().StartEventLoopTask();

__block CHIP_ERROR errorCode = CHIP_NO_ERROR;
Expand Down
4 changes: 4 additions & 0 deletions src/darwin/Framework/CHIP/MTRFramework.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#import "MTRFramework.h"
#import "MTRMetricsCollector.h"

#include <dispatch/dispatch.h>
#include <lib/support/CHIPMem.h>
Expand All @@ -34,5 +35,8 @@ void MTRFrameworkInit()
// Suppress CHIP logging until we actually need it for redirection
// (see MTRSetLogCallback()). Logging to os_log is always enabled.
chip::Logging::SetLogFilter(chip::Logging::kLogCategory_None);

// Startup metrics collection and tracing framework
StartupMetricsCollection();
});
}
16 changes: 15 additions & 1 deletion src/darwin/Framework/CHIP/MTRMetrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,28 @@
NS_ASSUME_NONNULL_BEGIN

/**
* A representation of metrics data for an operation.
* A representation of a collection of metrics data for an operation.
*/
MTR_NEWLY_AVAILABLE
@interface MTRMetrics : NSObject

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

/**
* @brief Returns the names of all the metrics data items collected.
*/
@property (nonatomic, readonly, copy) NSArray<NSString *> * allKeys;

/**
* @brief Returns metric object corresponding to the metric identified by its key
*
* @param [in] key Name of the metric
*
* @return An object containing the metric data, nil if key is invalid
*/
- (nullable id)valueForKey:(NSString *)key;

@end

NS_ASSUME_NONNULL_END
10 changes: 9 additions & 1 deletion src/darwin/Framework/CHIP/MTRMetrics.mm
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,24 @@
*/
#import "MTRLogging_Internal.h"
#import "MTRMetrics_Internal.h"
#include <Foundation/Foundation.h>
#import <Matter/MTRDefines.h>
#include <Matter/MTRMetrics.h>

@implementation MTRMetrics {
NSMutableDictionary<NSString *, id> * _metricsData;
}

- (instancetype)init
{
NSAssert(false, @"'init' unavailable, use initWithCapacity: instead");
return nil;
}

- (instancetype)initWithCapacity:(NSUInteger)numItems
{
if (self = [super init]) {
_metricsData = [NSMutableDictionary dictionary];
_metricsData = [NSMutableDictionary dictionaryWithCapacity:numItems];
}
return self;
}
Expand Down
59 changes: 59 additions & 0 deletions src/darwin/Framework/CHIP/MTRMetricsCollector.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
*
* Copyright (c) 2024 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#include <MTRDefines.h>
#import <Matter/MTRMetrics.h>

NS_ASSUME_NONNULL_BEGIN

/**
* This function initializes any backend required to collect metrics data.
*/
void StartupMetricsCollection();

/**
* This function shuts down any backend created to collect metrics data.
*/
void ShutdownMetricsCollection();

/**
* A representation of metrics data for an operation.
*/
@interface MTRMetricsCollector : NSObject

- (instancetype)init NS_UNAVAILABLE;
+ (instancetype)new NS_UNAVAILABLE;

/**
* Return the singleton MTRMetricsCollector to vend MTRMetrics snapshots
*/
+ (instancetype)sharedInstance;

/**
* @brief This method creates a snapshot of the metrics collected until the current point in time
* and returns an object with the stats.
*
* @param [in] resetCollection Boolean that specifies whether or not to clear the stats collected after
* creating the snapshot.
*
* @return MTRMetric object representing the metric data.
*/
- (MTRMetrics *)metricSnapshot:(BOOL)resetCollection;

@end

NS_ASSUME_NONNULL_END
Loading

0 comments on commit 8f2a4d2

Please sign in to comment.