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.

Sign up for GitHub

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

Enhance cron job configuration with timezone and retry options #3750

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from
+237 −140
Open

Enhance cron job configuration with timezone and retry options #3750

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4c3fb37
Enhance cron job configuration with timezone and retry options
Istiak-A-Tashrif Oct 23, 2024
f7e3cf2
query issue fixed
Istiak-A-Tashrif Oct 23, 2024
9534f9f
zero validation added
Istiak-A-Tashrif Oct 23, 2024
8e23385
modified timezone
Istiak-A-Tashrif Oct 24, 2024
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
195 changes: 111 additions & 84 deletions src/device-registry/bin/jobs/check-active-statuses.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,111 +5,138 @@ const logger = log4js.getLogger(
);
const DeviceModel = require("@models/Device");
const cron = require("node-cron");
const moment = require("moment-timezone"); // Import moment-timezone
Istiak-A-Tashrif marked this conversation as resolved.
Show resolved Hide resolved
const ACTIVE_STATUS_THRESHOLD = 0;
const { logText, logObject } = require("@utils/log");

// Configure timezone (e.g., 'UTC')
const TIMEZONE = 'UTC'; // You can change this to your desired timezone
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this!
Can we consider using moment.tz.guess() for this? More like:
const TIMEZONE = moment.tz.guess();

This is just to ensure consistent and accurate timezone handling.

const MAX_RETRIES = 3; // Maximum retry attempts
const RETRY_DELAY = 5000; // Delay between retries in milliseconds
Istiak-A-Tashrif marked this conversation as resolved.
Show resolved Hide resolved

const checkActiveStatuses = async () => {
try {
// Check for Deployed devices with incorrect statuses
const activeIncorrectStatusCount = await DeviceModel(
"airqo"
).countDocuments({
isActive: true,
status: { $ne: "deployed" },
});
for (let attempt = 1; attempt <= MAX_RETRIES; attempt++) {
try {
// Check for Deployed devices with incorrect statuses
const activeIncorrectStatusCount = await DeviceModel("airqo").countDocuments({
isActive: true,
status: { $ne: "deployed" },
});

const activeMissingStatusCount = await DeviceModel("airqo").countDocuments({
isActive: true,
status: { $exists: false } || { $eq: null } || { $eq: "" },
});
const activeMissingStatusCount = await DeviceModel("airqo").countDocuments({
isActive: true,
status: { $exists: false } || { $eq: null } || { $eq: "" },
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Correct the MongoDB query for status conditions

In your MongoDB queries, the use of JavaScript logical OR operators (||) does not function as intended. MongoDB queries require the use of the $or operator to combine multiple conditions.

Apply this diff to fix the query conditions:

For lines 28~ and 49~:

- status: { $exists: false } || { $eq: null } || { $eq: "" },
+ $or: [
+   { status: { $exists: false } },
+   { status: null },
+   { status: "" }
+ ],

This change ensures that the query correctly identifies documents where the status field is missing, null, or an empty string.

Also applies to: 49-49

});

const activeIncorrectStatusResult = await DeviceModel("airqo").aggregate([
{
$match: {
isActive: true,
status: { $ne: "deployed" },
const activeIncorrectStatusResult = await DeviceModel("airqo").aggregate([
{
$match: {
isActive: true,
status: { $ne: "deployed" },
},
},
},
{
$group: {
_id: "$name",
{
$group: {
_id: "$name",
},
},
},
]);
]);

const activeMissingStatusResult = await DeviceModel("airqo").aggregate([
{
$match: {
isActive: true,
status: { $exists: false } || { $eq: null } || { $eq: "" },
const activeMissingStatusResult = await DeviceModel("airqo").aggregate([
{
$match: {
isActive: true,
status: { $exists: false } || { $eq: null } || { $eq: "" },
},
},
},
{
$group: {
_id: "$name",
{
$group: {
_id: "$name",
},
},
},
]);

const activeIncorrectStatusUniqueNames = activeIncorrectStatusResult.map(
(doc) => doc._id
);
const activeMissingStatusUniqueNames = activeMissingStatusResult.map(
(doc) => doc._id
);
]);

logObject("activeIncorrectStatusCount", activeIncorrectStatusCount);
logObject("activeMissingStatusCount", activeMissingStatusCount);
const totalActiveDevices = await DeviceModel("airqo").countDocuments({
isActive: true,
});
const activeIncorrectStatusUniqueNames = activeIncorrectStatusResult.map(
(doc) => doc._id
);
const activeMissingStatusUniqueNames = activeMissingStatusResult.map(
(doc) => doc._id
);

const percentageActiveIncorrectStatus =
(activeIncorrectStatusCount / totalActiveDevices) * 100;
const percentageActiveMissingStatus =
(activeMissingStatusCount / totalActiveDevices) * 100;
logObject("activeIncorrectStatusCount", activeIncorrectStatusCount);
logObject("activeMissingStatusCount", activeMissingStatusCount);
const totalActiveDevices = await DeviceModel("airqo").countDocuments({
isActive: true,
});

logObject(
"percentageActiveIncorrectStatus",
percentageActiveIncorrectStatus
);
logObject("percentageActiveMissingStatus", percentageActiveMissingStatus);
const percentageActiveIncorrectStatus =
(activeIncorrectStatusCount / totalActiveDevices) * 100;
const percentageActiveMissingStatus =
(activeMissingStatusCount / totalActiveDevices) * 100;

if (
percentageActiveIncorrectStatus > ACTIVE_STATUS_THRESHOLD ||
percentageActiveMissingStatus > ACTIVE_STATUS_THRESHOLD
) {
logText(
`⁉️ Deployed devices with incorrect statuses (${activeIncorrectStatusUniqueNames.join(
", "
)}) - ${percentageActiveIncorrectStatus.toFixed(2)}%`
);
logger.info(
`⁉️ Deployed devices with incorrect statuses (${activeIncorrectStatusUniqueNames.join(
", "
)}) - ${percentageActiveIncorrectStatus.toFixed(2)}%`
logObject(
"percentageActiveIncorrectStatus",
percentageActiveIncorrectStatus
);
logObject("percentageActiveMissingStatus", percentageActiveMissingStatus);

logText(
`⁉️ Deployed devices missing status (${activeMissingStatusUniqueNames.join(
", "
)}) - ${percentageActiveMissingStatus.toFixed(2)}%`
);
logger.info(
`⁉️ Deployed devices missing status (${activeMissingStatusUniqueNames.join(
", "
)}) - ${percentageActiveMissingStatus.toFixed(2)}%`
);
if (
percentageActiveIncorrectStatus > ACTIVE_STATUS_THRESHOLD ||
percentageActiveMissingStatus > ACTIVE_STATUS_THRESHOLD
) {
logText(
`⁉️ Deployed devices with incorrect statuses (${activeIncorrectStatusUniqueNames.join(
", "
)}) - ${percentageActiveIncorrectStatus.toFixed(2)}%`
);
logger.info(
`⁉️ Deployed devices with incorrect statuses (${activeIncorrectStatusUniqueNames.join(
", "
)}) - ${percentageActiveIncorrectStatus.toFixed(2)}%`
);

logText(
`⁉️ Deployed devices missing status (${activeMissingStatusUniqueNames.join(
", "
)}) - ${percentageActiveMissingStatus.toFixed(2)}%`
);
logger.info(
`⁉️ Deployed devices missing status (${activeMissingStatusUniqueNames.join(
", "
)}) - ${percentageActiveMissingStatus.toFixed(2)}%`
);
}
break; // Exit loop if successful
} catch (error) {
logText(`🐛🐛 Error checking active statuses (Attempt ${attempt}): ${error.message}`);
logger.error(`🐛🐛 Error checking active statuses (Attempt ${attempt}): ${error.message}`);
logger.error(`🐛🐛 Stack trace: ${error.stack}`);

if (attempt < MAX_RETRIES) {
logger.warn(`Retry attempt ${attempt} due to error: ${error.message}`);
await new Promise((resolve) => setTimeout(resolve, RETRY_DELAY)); // Wait before retrying
} else {
logText("Maximum retry attempts reached. Job failed.");
logger.error("Maximum retry attempts reached. Job failed.");
}
}
} catch (error) {
logText(`🐛🐛 Error checking active statuses: ${error.message}`);
logger.error(`🐛🐛 Error checking active statuses: ${error.message}`);
logger.error(`🐛🐛 Stack trace: ${error.stack}`);
}
};

// Schedule job with timezone configuration
logText("Active statuses job is now running.....");
const schedule = "30 */2 * * *"; // At minute 30 of every 2nd hour
cron.schedule(schedule, checkActiveStatuses, {
const schedule = '30 */2 * * *'; // At minute 30 of every 2nd hour
cron.schedule(schedule, async () => {
const currentTime = moment.tz(TIMEZONE).format('YYYY-MM-DD HH:mm:ss');
logger.info(`Running job at ${currentTime}`);
await checkActiveStatuses();
}, {
scheduled: true,
timezone: TIMEZONE, // Set the desired timezone
Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet! I like the timezone aspect, much welcome. Thanks @Istiak-A-Tashrif

retry: {
retries: MAX_RETRIES,
onRetry: (error, attempt) => {
logger.warn(`Retry attempt ${attempt} due to error: ${error.message}`);
}
}
Istiak-A-Tashrif marked this conversation as resolved.
Show resolved Hide resolved
});