Dns addition in monitor#3449
Conversation
Owaiseimdad
requested review from
Br0wnHammer,
ajhollid and
karenvicent
as code owners
|
Hey @Owaiseimdad, Let's split this up into two PRs please, one for frontend and one for backend. We'll review backend functionality for correctness first, then we can look at the UI side of of things. Thanks! |
I have added only backend now and after this will raise a UI related code. |
Br0wnHammer
left a comment
Br0wnHammer
left a comment
There was a problem hiding this comment.
Hey @Owaiseimdad , nice work adding DNS monitoring support. I left a few comments on DnsProvider.ts. Please take a look.
| import { Monitor, MonitorType } from "@/types/monitor.js"; | ||
| import { NETWORK_ERROR } from "@/service/infrastructure/network/utils.js"; | ||
|
|
||
| export class DnsProvider implements IStatusProvider<DnsStatusPayload> { |
There was a problem hiding this comment.
DnsProvider doesn't follow the project's dependency injection pattern. DnsProvider directly imports Resolver from dns/promises and instantiates it inside handle().
| throw new Error("Hostname is required for DNS monitor"); | ||
| } | ||
|
|
||
| const resolver = new Resolver(); |
There was a problem hiding this comment.
Other providers enforce a timeout (e.g. TIMEOUT_MS = 10000) and DNS queries can hang indefinitely without one.
| resolver.setServers([dnsServer]); | ||
| } | ||
|
|
||
| const startTime = process.hrtime(); |
There was a problem hiding this comment.
Other providers use the shared timeRequest helper from utils.js instead of manually calling hrtime()
| return type === "dns"; | ||
| } | ||
|
|
||
| async handle(monitor: Monitor): Promise<MonitorStatusResponse<DnsStatusPayload>> { |
There was a problem hiding this comment.
The entire method body needs an outer try/catch that wraps unexpected errors in AppError (with service, method, details), matching the pattern in WebSocketProvider and GrpcProvider.
5 participants
Describe your changes
Implemented a new DNS Monitor type to allow users to track domain resolution success and query latency. Key changes include:
Core Logic: Created DnsProvider.ts using Node's dns/promises API. It supports configurable DNS servers and multiple record types (A, AAAA, CNAME, MX, TXT, NS).
Backend Persistence:
Updated the Mongoose Monitor schema to include dnsServer and dnsRecordType.
Updated both MongoMonitorsRepository and TimescaleMonitorsRepository to ensure DNS configuration persists across both database types. Updated MonitorService to support fetching uptime details for the new DNS type.
Validation: Refactored the backend validation to use the centralized MonitorTypes constant in monitorValidation.ts and checkValidation.ts, resolving Zod validation errors for the new type.
Frontend UI:
Added DNS-specific fields (DNS Server, Record Type) to the CreateMonitorPage.
Updated the monitor list and ControlsFilter to include DNS monitors by default.
Improved UI consistency by rounding both Average and Maximum latency values to the nearest millisecond in
HistogramResponseTime.tsx.
i18n: Added comprehensive English translations for all new DNS-related labels and descriptions in en.json
.
Write your issue number after "Fixes "
#3388
Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.
<div>Add</div>, use):npm run formatin server and client directories, which automatically formats your code.