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

Update Device Defender demo to send custom metrics #1547

Merged
merged 27 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
6469599
Bump submodule of Device Defender
aggarw13 Feb 18, 2021
410251b
Update Defender demo to send CPU usage data as number list custom metric
aggarw13 Feb 18, 2021
1631d97
Update metrics collector to generically support custom metric types t…
aggarw13 Feb 18, 2021
bb56434
Add support for IP address list type of custom metric
aggarw13 Feb 18, 2021
657aea2
Fixes in report builder to support non-number list metric types; add …
aggarw13 Feb 18, 2021
d2e6d9f
Hygiene improvements
aggarw13 Feb 18, 2021
f99c8fb
Fix issue of missing comma in JSON report for number list type of cu…
aggarw13 Feb 18, 2021
a74e9b0
Add docstring for private function in report builder
aggarw13 Feb 19, 2021
4e9080d
style: make API naming and implementation consistent for metrics coll…
aggarw13 Feb 19, 2021
a103ba8
more style changes
aggarw13 Feb 19, 2021
382cd8b
Fix spellings and spell-check failure
aggarw13 Feb 19, 2021
c132c55
Change memory statistic custom metric to only send memory values inst…
aggarw13 Feb 19, 2021
c4fd4fc
Refactor custom metrics logic to be specific to demo application for …
aggarw13 Feb 22, 2021
6e432ba
Merge remote-tracking branch 'origin/main' into defender-demo/add-cus…
aggarw13 Feb 22, 2021
573462e
Fix spellcheck
aggarw13 Feb 22, 2021
651d882
Change data type for CPU usage time to uint32_t
aggarw13 Feb 23, 2021
9a689a9
Add line about device defender demo update in CHANGELOG.md
aggarw13 Feb 23, 2021
5f50723
style: improvements from review feedback
aggarw13 Feb 23, 2021
e4f4116
Hygiene improvements from code reviews
aggarw13 Feb 23, 2021
469c03e
Fix spell check
aggarw13 Feb 23, 2021
2293a33
style: more improvements from feedback
aggarw13 Feb 24, 2021
d5a35a6
More hygiene changes from review comments
aggarw13 Feb 24, 2021
a1e8d1d
Merge branch 'defender-demo/add-custom-metrics' of github.com:aggarw1…
aggarw13 Feb 24, 2021
06512bb
Fix stale comment
aggarw13 Feb 24, 2021
e84f7d5
Use macros from Defender library for key names in JSON report
aggarw13 Feb 24, 2021
f8008de
Fix spelling
aggarw13 Feb 24, 2021
834f591
Replace remaining report key with library macro
aggarw13 Feb 24, 2021
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
43 changes: 42 additions & 1 deletion demos/defender/defender_demo_json/defender_demo.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,15 @@
* @brief Time in seconds to wait between retries of the demo loop if
* demo loop fails.
*/
#define DELAY_BETWEEN_DEMO_RETRY_ITERATIONS_S ( 5 )
#define DELAY_BETWEEN_DEMO_RETRY_ITERATIONS_S ( 5 )

/**
* @brief Number of custom metrics that will be sent to AWS IoT Device Defender service by
* the demo.
* This demo sends custom metrics for Cpu usage data (as number-list type) and memory data
* (as string-list type).
*/
#define NUMBER_OF_CUSTOM_METRICS_OBJECTS_IN_JSON_REPORT ( 2 )
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief Status values of the device defender report.
Expand Down Expand Up @@ -135,6 +143,12 @@ static uint16_t openUdpPorts[ OPEN_UDP_PORTS_ARRAY_SIZE ];
*/
static Connection_t establishedConnections[ ESTABLISHED_CONNECTIONS_ARRAY_SIZE ];

/**
* @brief Memory to represent custom metrics of CPU usage time and memory statistics
* that this demo sends to the AWS IoT Defender service.
*/
static CustomMetrics_t customMetrics;

/**
* @brief All the metrics sent in the device defender report.
*/
Expand Down Expand Up @@ -410,6 +424,32 @@ static bool collectDeviceMetrics( void )
}
}

/* Collect CPU usage time metrics from the system.
* This is an example of a custom metric of number-list type. */
if( metricsCollectorStatus == MetricsCollectorSuccess )
{
metricsCollectorStatus = GetCpuUsageStats( &customMetrics.cpuUsageStats );
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved

if( metricsCollectorStatus != MetricsCollectorSuccess )
{
LogError( ( "Failed to get data of CPU usage from system. Status: %d.",
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
metricsCollectorStatus ) );
}
}

/* Collect data on memory usage and calculate statistic on % of available memory
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
* in the system. This is an example of a custom metric of string-list type. */
if( metricsCollectorStatus == MetricsCollectorSuccess )
{
metricsCollectorStatus = GetMemoryStats( &customMetrics.memoryStats );
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved

if( metricsCollectorStatus != MetricsCollectorSuccess )
{
LogError( ( "Failed to get data of memory statistics from system. Status: %d.",
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
metricsCollectorStatus ) );
}
}

/* Populate device metrics. */
if( metricsCollectorStatus == MetricsCollectorSuccess )
{
Expand All @@ -421,6 +461,7 @@ static bool collectDeviceMetrics( void )
deviceMetrics.openUdpPortsArrayLength = numOpenUdpPorts;
deviceMetrics.pEstablishedConnectionsArray = &( establishedConnections[ 0 ] );
deviceMetrics.establishedConnectionsArrayLength = numEstablishedConnections;
deviceMetrics.pCustomMetrics = &customMetrics;
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
}

return status;
Expand Down
6 changes: 2 additions & 4 deletions demos/defender/defender_demo_json/demo_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
#endif

#ifndef LIBRARY_LOG_LEVEL
#define LIBRARY_LOG_LEVEL LOG_INFO
#define LIBRARY_LOG_LEVEL LOG_DEBUG
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
#endif

#include "logging_stack.h"
Expand Down Expand Up @@ -194,10 +194,8 @@

/**
* @brief Size of the buffer which contains the generated device defender report.
*
* If the generated report is larger than this, it is rejected.
*/
#define DEVICE_METRICS_REPORT_BUFFER_SIZE 1000
#define DEVICE_METRICS_REPORT_BUFFER_SIZE 2000

/**
* @brief Major version number of the device defender report.
Expand Down
152 changes: 150 additions & 2 deletions demos/defender/defender_demo_json/metrics_collector.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@
#include "metrics_collector.h"

/**
* @brief The maximum length of line read from any of /proc/net/dev, /proc/net/tcp
* and /proc/net/udp.
* @brief The maximum length of line read from any of /proc/net/dev, /proc/net/tcp,
* /proc/net/udp, /proc/uptime and /proc/meminfo files.
*/
#define MAX_LINE_LENGTH ( 256 )

Expand Down Expand Up @@ -378,4 +378,152 @@ MetricsCollectorStatus_t GetEstablishedConnections( Connection_t * pOutConnectio

return status;
}

/*-----------------------------------------------------------*/

MetricsCollectorStatus_t GetCpuUsageStats( CpuUsageStats_t * pCpuUsage )
{
MetricsCollectorStatus_t status = MetricsCollectorSuccess;
FILE * fileHandle = NULL;
uint32_t filledVariables;
char lineBuffer[ MAX_LINE_LENGTH ];

if( pCpuUsage == NULL )
{
LogError( ( "Invalid parameter. pCpuUsage %p", ( void * ) pCpuUsage ) );
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
status = MetricsCollectorBadParameter;
}

if( status == MetricsCollectorSuccess )
{
fileHandle = fopen( "/proc/uptime", "r" );

if( fileHandle == NULL )
{
LogError( ( "Failed to open /proc/uptime." ) );
status = MetricsCollectorFileOpenFailed;
}
}

if( status == MetricsCollectorSuccess )
{
if( fgets( &( lineBuffer[ 0 ] ), MAX_LINE_LENGTH, fileHandle ) != NULL )
{
float upTime = 0.0f, idleTime = 0.0f;
LogDebug( ( "File: /proc/uptime, Content: %s.",
&( lineBuffer[ 0 ] ) ) );

/* Parse the output. */
filledVariables = sscanf( &( lineBuffer[ 0 ] ),
"%f %f",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to use float here? The code that outputs these numbers utilizes %lu: https://elixir.bootlin.com/linux/v4.0/source/fs/proc/uptime.c#L27

            filledVariables = sscanf( &( lineBuffer[ 0 ] ),
                                      "%lu.%*u %lu.%*u",
                                      &( upTime ),
                                      &( idleTime ) );

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use integer type then for the non-decimal part of the value.
Based on the source code link, we may have to ideally use struct timespec (to store higher precision of data) type but for simplicity of reporting only the seconds part of the time, we can use integer type.

&( upTime ),
&( idleTime ) );

/* sscanf should fill all the 2 variables successfully. */
if( filledVariables != 2 )
{
LogError( ( "Failed to CPU usage data. File: /proc/uptime, Data: %s.", &( lineBuffer[ 0 ] ) ) );
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
status = MetricsCollectorParsingFailed;
}
else
{
/* Convert data from floating point to integer by multiplying by 100 to represent data in
* USER_HZ time units. */
pCpuUsage->upTime = ( int64_t ) ( upTime * 100.0f );
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
pCpuUsage->idleTime = ( int64_t ) ( idleTime * 100.0f );
}
}
}

if( fileHandle != NULL )
{
fclose( fileHandle );
}

return status;
}
/*-----------------------------------------------------------*/

MetricsCollectorStatus_t GetMemoryStats( MemoryStats_t * pMemoryData )
{
MetricsCollectorStatus_t status = MetricsCollectorSuccess;
FILE * fileHandle = NULL;
/* Variables for reading and processing data from "/proc/meminfo" file. */
uint8_t lineNumber = 0;
char lineBuffer[ MAX_LINE_LENGTH ];
bool readTotalMem = false, readAvailableMem = false;
const char * const pTotalMemoryField = "MemTotal";
const char * const pAvailableMemField = "MemAvailable";
int filledVariables = 0;
uint64_t parsedMemData = 0UL;

if( ( pMemoryData == NULL ) )
{
LogError( ( "Invalid parameter. pMemoryData=%p", ( void * ) pMemoryData ) );
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
status = MetricsCollectorBadParameter;
}

if( status == MetricsCollectorSuccess )
{
fileHandle = fopen( "/proc/meminfo", "r" );

if( fileHandle == NULL )
{
LogError( ( "Failed to open /proc/meminfo." ) );
status = MetricsCollectorFileOpenFailed;
}
}

if( status == MetricsCollectorSuccess )
{
while( ( fgets( &( lineBuffer[ 0 ] ), MAX_LINE_LENGTH, fileHandle ) != NULL ) )
{
lineNumber++;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lineNumber is not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is being printed in the debug and error log messages while lines are read from proc/meminfo just like other functions read multiple lines

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other function that has lineNumber is GetEstablishedConnections which uses it to skip the first line.


LogDebug( ( "File: /proc/meminfo, LineNo: %u, Content: %s.",
lineNumber, &( lineBuffer[ 0 ] ) ) );

/* Extract the total memory value from the line. */
filledVariables = sscanf( lineBuffer,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done only for the lines starting with MemTotal and MemAvailable.

Copy link
Contributor Author

@aggarw13 aggarw13 Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That leads to duplication of the sscanf logic in the if-else-if branches that match for the field names. I have pushed the change though as I don't have a strong preference for either.

"%*[^1-9]%lu kB",
( &parsedMemData ) );

if( filledVariables != 1 )
{
LogError( ( "Failed to parse data. File: /proc/meminfo, Line:%d, Content:%s", lineNumber, lineBuffer ) );
status = MetricsCollectorParsingFailed;
}
/* Check if the line read represents information for "total memory" in the system. */
else if( strncmp( lineBuffer, pTotalMemoryField, strlen( pTotalMemoryField ) ) == 0 )
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
{
/* Populate buffer with information for total memory as "<data>kB". */
( void ) snprintf( pMemoryData->totalMemory, sizeof( pMemoryData->totalMemory ),
"%lukB", parsedMemData );

readTotalMem = true;
}
/* Check if the line read represents information for "available memory" in the system. */
else if( strncmp( lineBuffer, pAvailableMemField, strlen( pAvailableMemField ) ) == 0 )
{
/* Populate buffer with information for total memory as "<data>kB". */
( void ) snprintf( pMemoryData->availableMemory, sizeof( pMemoryData->availableMemory ),
"%lukB", parsedMemData );

readAvailableMem = true;
}

if( readTotalMem && readAvailableMem )
{
/* We have obtained data for both total and available memory in the system. */
break;
}
}
}

aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
if( fileHandle != NULL )
{
fclose( fileHandle );
}

return status;
}
53 changes: 53 additions & 0 deletions demos/defender/defender_demo_json/metrics_collector.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,28 @@ typedef struct Connection
uint16_t remotePort;
} Connection_t;

/**
* @brief Represents the Cpu Usage statistics obtained from "/proc/uptime".
* Refer to Linux manual for "/proc" filesystem for more information.
* https://man7.org/linux/man-pages/man5/procfs.5.html
*/
typedef struct CpuUsageData
{
uint32_t upTime; /**< Up-time of system in USER_HZ (1/100th of second) time units. */
uint32_t idleTime; /**< Idle time of system in USER_HZ (1/100th of second) time units. */
} CpuUsageStats_t;

/**
* @brief Represents the memory data of total and available memory from "/proc/uptime".
* Refer to Linux manual for "/proc" filesystem for more information.
* https://man7.org/linux/man-pages/man5/procfs.5.html
*/
typedef struct MemoryStats
{
char totalMemory[ 50 ]; /**< Amount of total memory in system (in kB). */
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
char availableMemory[ 50 ]; /**< Amount of available memory in system (in kB). */
} MemoryStats_t;

/**
* @brief Get network stats.
*
Expand Down Expand Up @@ -145,4 +167,35 @@ MetricsCollectorStatus_t GetEstablishedConnections( Connection_t * pOutConnectio
uint32_t connectionsArrayLength,
uint32_t * pOutNumEstablishedConnections );

/**
* @brief Get CPU usage data of uptime and idle time from the system.
*
* This function finds the system CPU information by reading the "/proc/uptime" file.
*
* @param[out] pCpuUsage The memory to write the CPU usage statistics into.
*
* @return #MetricsCollectorSuccess if CPU usage data is successfully obtained;
* #MetricsCollectorBadParameter if invalid parameter is passed;
* #MetricsCollectorFileOpenFailed if the function fails to open "/proc/uptime";
* MetricsCollectorParsingFailed if the function fails to parses the data read
* from "/proc/uptime".
*/
MetricsCollectorStatus_t GetCpuUsageStats( CpuUsageStats_t * pCpuUsage );

/**
* @brief Gets data of total and available memory in the system and populates them as
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
* key-value strings in the passed @p pMemoryData parameter.
*
* This function finds the memory information by reading the "/proc/meminfo" file.
*
* @param[out] pMemoryData The memory to write the memory information into.
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved
*
* @return #MetricsCollectorSuccess if memory data statistic is successfully calculated;
* #MetricsCollectorBadParameter if invalid parameter is passed;
* #MetricsCollectorFileOpenFailed if the function fails to open "/proc/meminfo";
* MetricsCollectorParsingFailed if the function fails to parses the data read
* from "/proc/meminfo".
*/
MetricsCollectorStatus_t GetMemoryStats( MemoryStats_t * pMemoryData );
aggarw13 marked this conversation as resolved.
Show resolved Hide resolved

#endif /* ifndef METRICS_COLLECTOR_H_ */
Loading