feat: improve telemetry ISE logging and diagnostics#100
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves telemetry ISE observability by adding more explicit error logging around key failure paths in telemetry rule handlers/services (notably XResponseWriter casting failures and DB persistence failures), making production triage and log correlation easier.
Changes:
- Added error logs when
http.ResponseWritercannot be cast to*xwhttp.XResponseWriterin several telemetry handlers. - Added structured error logs for telemetry rule delete/create/update failures when persisting to the DB.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
adminapi/telemetry/telemetry_v2_rule_handler.go |
Adds a cast-failure log in paginated v2 rules handler. |
adminapi/telemetry/telemetry_rule_service.go |
Adds structured logs on DB failure paths for delete/create/update. |
adminapi/telemetry/telemetry_rule_handler.go |
Adds a cast-failure log in the telemetry rule update handler. |
adminapi/telemetry/telemetry_profile_controller.go |
Adds a cast-failure log in telemetry entry creation flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| xw, ok := w.(*xwhttp.XResponseWriter) | ||
| if !ok { | ||
| log.Error("Failed to cast response writer to XResponseWriter in GetTelemetryTwoRulesFilteredWithPage") |
There was a problem hiding this comment.
The new cast-failure log message uses different wording than the other XResponseWriter cast logs in this file (e.g., "Unable to cast responsewriter..."). Consider standardizing the message text and also logging the concrete type of w (e.g., via a field like writer_type) to make this diagnostic actionable.
| log.Error("Failed to cast response writer to XResponseWriter in GetTelemetryTwoRulesFilteredWithPage") | |
| log.WithField("writer_type", fmt.Sprintf("%T", w)). | |
| Error("Unable to cast responsewriter to XResponseWriter in GetTelemetryTwoRulesFilteredWithPage") |
| // r.Body is already drained in the middleware | ||
| xw, ok := w.(*xwhttp.XResponseWriter) | ||
| if !ok { | ||
| log.Error("failed to cast responsewriter to XResponseWriter in UpdateTelemetryRuleHandler") |
There was a problem hiding this comment.
This log message starts with a lowercase "failed" and differs from the other cast-failure messages ("Unable to cast responsewriter...") used in nearby telemetry handlers. Standardizing casing/wording makes grepping and alerting on these diagnostics more reliable.
| log.Error("failed to cast responsewriter to XResponseWriter in UpdateTelemetryRuleHandler") | |
| log.Error("Unable to cast responsewriter to XResponseWriter in UpdateTelemetryRuleHandler") |
|
|
||
| err = DeleteOneTelemetryRule(id) | ||
| if err != nil { | ||
| log.WithFields(log.Fields{"entity_id": id, "error": err}).Error("Failed to delete telemetry rule") |
There was a problem hiding this comment.
To improve triage across tenants/environments, consider including app/ApplicationType as a structured field in this error log (it is available as a parameter here).
| log.WithFields(log.Fields{"entity_id": id, "error": err}).Error("Failed to delete telemetry rule") | |
| log.WithFields(log.Fields{"entity_id": id, "app": app, "error": err}).Error("Failed to delete telemetry rule") |
| tmrule.Updated = xwutil.GetTimestamp() | ||
| if err := db.GetCachedSimpleDao().SetOne(db.TABLE_TELEMETRY_RULES, tmrule.ID, tmrule); err != nil { | ||
| log.WithFields(log.Fields{"entity_id": tmrule.ID, "error": err}).Error("Failed to save telemetry rule") | ||
| return xwhttp.NewResponseEntity(http.StatusInternalServerError, err, nil) | ||
| } |
There was a problem hiding this comment.
To improve triage across tenants/environments, consider including app/ApplicationType as a structured field in this error log (it is available as a parameter here).
| tmrule.Updated = xwutil.GetTimestamp() | ||
| if err = db.GetCachedSimpleDao().SetOne(db.TABLE_TELEMETRY_RULES, tmrule.ID, tmrule); err != nil { | ||
| log.WithFields(log.Fields{"entity_id": tmrule.ID, "error": err}).Error("Failed to update telemetry rule") | ||
| return xwhttp.NewResponseEntity(http.StatusInternalServerError, err, nil) |
There was a problem hiding this comment.
To improve triage across tenants/environments, consider including app/ApplicationType as a structured field in this error log (it is available as a parameter here).
feat(telemetry): improve ISE logging and diagnostics
Summary
This PR enhances telemetry ISE observability by improving logging quality and adding clearer diagnostic signals.
What changed
Why
Impact