Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements significant data observability enhancements to support time-based metrics tracking and system downtime monitoring. The changes expand the metrics service to handle both hourly and daily granularity data aggregation, introduce infrastructure monitoring capabilities, and enhance the API to support flexible date range queries.
- Enhanced metrics collection with hourly/daily granularity support and status tracking over time
- Added comprehensive downtime monitoring with Prometheus integration for Kubernetes pod health
- Replaced data quality metrics with infrastructure downtime tracking capabilities
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| DatasetMetricsService.ts | Major refactor to support time-series data with hourly/daily granularity and added downtime monitoring |
| queries.ts | Updated query functions to support time periods and added new Prometheus queries for extractor metrics |
| DatasetMetricsController.ts | Added date range validation and switched from data_quality to down_time category |
| DatasetMetrics.json | Updated schema to support date ranges and downtime configuration |
| DataObsrvDefaults.ts | New configuration file defining container monitoring setup |
| if (totalEvents > 0) { | ||
| const failurePercentage = (failedEvents / totalEvents) * 100; | ||
| status = failurePercentage > 5 ? "Unhealthy" : "Healthy"; | ||
| reason = status === "Unhealthy" ? "High events failure rate detected is higher than threshold" : "No issues reported"; |
There was a problem hiding this comment.
The error message contains redundant text. 'High events failure rate detected is higher than threshold' should be simplified to 'Events failure rate is higher than threshold' to avoid the redundant 'detected is higher' phrasing.
| reason = status === "Unhealthy" ? "High events failure rate detected is higher than threshold" : "No issues reported"; | |
| reason = status === "Unhealthy" ? "Events failure rate is higher than threshold" : "No issues reported"; |
| throw new Error(`Prometheus query failed: ${response.statusText}`); | ||
| } | ||
| return await response.json(); | ||
| } |
There was a problem hiding this comment.
Missing semicolon after the function declaration. JavaScript/TypeScript functions should end with a semicolon for consistency.
| } | |
| }; |
| for (let i = 0; i < 24; i++) { | ||
| const currentHour = startTime.add(i + 1, 'hour'); | ||
| const timestamp = currentHour.format('YYYY-MM-DDTHH:00:00.000Z'); | ||
| const processingTime: any = dataMap.get(timestamp) || 0; |
There was a problem hiding this comment.
Using 'any' type defeats TypeScript's type safety. Consider using 'number' type instead since the value is expected to be numeric (defaulting to 0).
| const processingTime: any = dataMap.get(timestamp) || 0; | |
| const processingTime: number = (dataMap.get(timestamp) as number) || 0; |
| // Fill in all days | ||
| while (currentDate.isBefore(lastDate)) { | ||
| const dateStr = currentDate.format('YYYY-MM-DD'); | ||
| const processingTime: any = dataMap.get(dateStr) || 0; |
There was a problem hiding this comment.
Using 'any' type defeats TypeScript's type safety. Consider using 'number' type instead since the value is expected to be numeric (defaulting to 0).
| const processingTime: any = dataMap.get(dateStr) || 0; | |
| const processingTime: number = dataMap.get(dateStr) || 0; |
| const totalEvents: any = eventsMap.get(timestamp) || 0; | ||
| const failedEvents: any = failedEventsMap.get(timestamp) || 0; |
There was a problem hiding this comment.
Using 'any' type defeats TypeScript's type safety. Consider using 'number' type instead since the value is expected to be numeric (defaulting to 0).
| const totalEvents: any = eventsMap.get(timestamp) || 0; | |
| const failedEvents: any = failedEventsMap.get(timestamp) || 0; | |
| const totalEvents: number = eventsMap.get(timestamp) || 0; | |
| const failedEvents: number = failedEventsMap.get(timestamp) || 0; |
| while (currentDate.isBefore(lastDate)) { | ||
| const dateStr = currentDate.format('YYYY-MM-DD'); | ||
| const totalEvents: any = eventsMap.get(dateStr) || 0; | ||
| const failedEvents: any = failedEventsMap.get(dateStr) || 0; |
There was a problem hiding this comment.
Using 'any' type defeats TypeScript's type safety. Consider using 'number' type instead since the value is expected to be numeric (defaulting to 0).
| const failedEvents: any = failedEventsMap.get(dateStr) || 0; | |
| const failedEvents: number = failedEventsMap.get(dateStr) || 0; |
| const totalEventsCount: any = Array.from(eventsMap.values()).reduce((sum: any, count) => sum + count, 0); | ||
| const totalFailedEventsCount: any = Array.from(failedEventsMap.values()).reduce((sum: any, count) => sum + count, 0); |
There was a problem hiding this comment.
Using 'any' type defeats TypeScript's type safety. Consider using 'number' type for both totalEventsCount and the sum parameter since they represent numeric values.
| const totalEventsCount: any = Array.from(eventsMap.values()).reduce((sum: any, count) => sum + count, 0); | |
| const totalFailedEventsCount: any = Array.from(failedEventsMap.values()).reduce((sum: any, count) => sum + count, 0); | |
| const totalEventsCount: number = Array.from(eventsMap.values()).reduce((sum: number, count: number) => sum + count, 0); | |
| const totalFailedEventsCount: number = Array.from(failedEventsMap.values()).reduce((sum: number, count: number) => sum + count, 0); |
| const totalEventsCount: any = Array.from(eventsMap.values()).reduce((sum: any, count) => sum + count, 0); | ||
| const totalFailedEventsCount: any = Array.from(failedEventsMap.values()).reduce((sum: any, count) => sum + count, 0); |
There was a problem hiding this comment.
Using 'any' type defeats TypeScript's type safety. Consider using 'number' type for both totalFailedEventsCount and the sum parameter since they represent numeric values.
| const totalEventsCount: any = Array.from(eventsMap.values()).reduce((sum: any, count) => sum + count, 0); | |
| const totalFailedEventsCount: any = Array.from(failedEventsMap.values()).reduce((sum: any, count) => sum + count, 0); | |
| const totalEventsCount: number = Array.from(eventsMap.values()).reduce((sum: number, count: number) => sum + count, 0); | |
| const totalFailedEventsCount: number = Array.from(failedEventsMap.values()).reduce((sum: number, count: number) => sum + count, 0); |
| })(); | ||
|
|
||
| const duration = time_period_str === "1" ? "1h" : "1d"; | ||
| const downtimeMetrics: any[] = []; |
There was a problem hiding this comment.
Using 'any[]' type defeats TypeScript's type safety. Consider defining a proper interface for downtime metrics with properties like container, componentType, totalDowntime, intervalStart, and status.
| const downtimeMetrics: any[] = []; | |
| const downtimeMetrics: DowntimeMetric[] = []; |
| const totalFailedEventsPayload = totalFailedEventsQuery(intervals, dataset_id); | ||
| const totalQueryCalls = generateTotalQueryCallsQuery(config?.data_observability?.data_out_query_time_period); | ||
| const totalQueryCallsAtDatasetLevel = generateDatasetQueryCallsQuery(dataset_id, config?.data_observability?.data_out_query_time_period); | ||
| export const getDataObservability = async (dataset_id: string, intervals: string, time_period: any) => { |
There was a problem hiding this comment.
Parameter 'time_period' uses 'any' type. Consider using 'number' type instead since it's used in numeric operations and comparisons.
| export const getDataObservability = async (dataset_id: string, intervals: string, time_period: any) => { | |
| export const getDataObservability = async (dataset_id: string, intervals: string, time_period: number) => { |
No description provided.