This is an automated email from the ASF dual-hosted git repository.
klesh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-devlake.git
The following commit(s) were added to refs/heads/main by this push:
new 048adfb4e Allow Rootly plugin to collect incidents without service
identifiers assigned (#8892)
048adfb4e is described below
commit 048adfb4e3a9c9fb9900cae2a5cad87e7bb71ab6
Author: Brian Feucht <[email protected]>
AuthorDate: Fri May 29 20:40:13 2026 -0700
Allow Rootly plugin to collect incidents without service identifiers
assigned (#8892)
* fix: map Rootly 'completed' status to DONE in incident converter
The mapStatus function was missing 'completed' as a terminal status,
causing incidents with that status to be written as IN_PROGRESS instead
of DONE.
Co-authored-by: Copilot <[email protected]>
* fix: use updated_date as MTTR fallback for Rootly 'completed' incidents
Rootly's 'completed' status represents a fully resolved incident that has
gone through post-incident review. These incidents may not have a
resolved_date populated. Fall back to updated_date so MTTR is computed
correctly instead of showing the incident as unresolved.
Co-authored-by: Copilot <[email protected]>
* fix: remove service filter requirement from Rootly incident collection
Allow collecting all incidents without filtering by service ID.
Previously, all incidents required a service association to be collected;
incidents created without service tags (common in Rootly) were silently
dropped.
Changes:
- ValidateTaskOptions: service_id is now optional (no longer required)
- GetParams: falls back to scope_id 'all' when service_id is empty
- buildIncidentsQuery: only adds filter[service_ids] when service_id is set
- extractRootlyIncident: skips the service-membership guard when
collecting globally (service_id = '') to avoid false drops
Co-authored-by: Copilot <[email protected]>
---------
Co-authored-by: Copilot <[email protected]>
---
backend/plugins/rootly/tasks/incidents_collector.go | 4 +++-
.../rootly/tasks/incidents_collector_test.go | 7 +++++++
backend/plugins/rootly/tasks/incidents_converter.go | 20 +++++++++++++-------
.../rootly/tasks/incidents_converter_test.go | 21 +++++++++++++++++----
backend/plugins/rootly/tasks/incidents_extractor.go | 9 ++++++---
backend/plugins/rootly/tasks/task_data.go | 9 +++++----
6 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/backend/plugins/rootly/tasks/incidents_collector.go
b/backend/plugins/rootly/tasks/incidents_collector.go
index e89bb89fc..652801217 100644
--- a/backend/plugins/rootly/tasks/incidents_collector.go
+++ b/backend/plugins/rootly/tasks/incidents_collector.go
@@ -126,7 +126,9 @@ func CollectIncidents(taskCtx plugin.SubTaskContext)
errors.Error {
// `filter[service_ids]`) is caught by a unit test.
func buildIncidentsQuery(serviceId string, pageSize, pageNumber int,
createdAfter *time.Time) url.Values {
query := url.Values{}
- query.Set("filter[service_ids]", serviceId)
+ if serviceId != "" {
+ query.Set("filter[service_ids]", serviceId)
+ }
query.Set("page[size]", fmt.Sprintf("%d", pageSize))
// Rootly's JSON:API pagination is 1-based.
query.Set("page[number]", fmt.Sprintf("%d", pageNumber))
diff --git a/backend/plugins/rootly/tasks/incidents_collector_test.go
b/backend/plugins/rootly/tasks/incidents_collector_test.go
index eb3c02415..3456749a4 100644
--- a/backend/plugins/rootly/tasks/incidents_collector_test.go
+++ b/backend/plugins/rootly/tasks/incidents_collector_test.go
@@ -34,6 +34,13 @@ func TestBuildIncidentsQuery_FirstPageNoSince(t *testing.T) {
assert.Equal(t, "", q.Get("filter[services]"), "regression guard: must
be filter[service_ids], not filter[services]")
}
+func TestBuildIncidentsQuery_NoServiceFilter(t *testing.T) {
+ q := buildIncidentsQuery("", 100, 1, nil)
+ assert.Equal(t, "", q.Get("filter[service_ids]"), "empty serviceId must
omit the service filter entirely")
+ assert.Equal(t, "100", q.Get("page[size]"))
+ assert.Equal(t, "1", q.Get("page[number]"))
+}
+
func TestBuildIncidentsQuery_SubsequentPage(t *testing.T) {
q := buildIncidentsQuery("svc_42", 100, 3, nil)
assert.Equal(t, "3", q.Get("page[number]"))
diff --git a/backend/plugins/rootly/tasks/incidents_converter.go
b/backend/plugins/rootly/tasks/incidents_converter.go
index 856575d78..daba86f50 100644
--- a/backend/plugins/rootly/tasks/incidents_converter.go
+++ b/backend/plugins/rootly/tasks/incidents_converter.go
@@ -90,7 +90,7 @@ func ConvertIncidents(taskCtx plugin.SubTaskContext)
errors.Error {
logger.Warn(nil, "unknown rootly incident
status: %s", incident.Status)
}
- leadTime, resolutionDate :=
computeLeadTime(incident.StartedDate, incident.ResolvedDate)
+ leadTime, resolutionDate :=
computeLeadTime(incident.StartedDate, incident.ResolvedDate,
incident.UpdatedDate, incident.Status)
domainIssueId :=
idGen.Generate(data.Options.ConnectionId, incident.Id)
@@ -161,7 +161,7 @@ func mapStatus(status string) (mapped string, known bool) {
return ticket.TODO, true
case "mitigated":
return ticket.IN_PROGRESS, true
- case "resolved", "closed", "cancelled":
+ case "resolved", "closed", "cancelled", "completed":
return ticket.DONE, true
default:
return ticket.IN_PROGRESS, false
@@ -183,18 +183,24 @@ func mapSeverityToPriority(severity string) string {
}
}
-func computeLeadTime(started time.Time, resolved *time.Time) (*uint,
*time.Time) {
- if resolved == nil {
+func computeLeadTime(started time.Time, resolved *time.Time, updated
time.Time, status string) (*uint, *time.Time) {
+ // For "completed" incidents Rootly may not populate resolved_date; fall
+ // back to updated_date which reflects when the incident was last
actioned.
+ effective := resolved
+ if effective == nil && status == "completed" {
+ effective = &updated
+ }
+ if effective == nil {
return nil, nil
}
// Clock skew / backfill can place resolved before started. A naive
// uint() cast on a negative duration wraps to huge garbage and
// silently corrupts MTTR; treat as unresolved instead.
- if resolved.Before(started) {
+ if effective.Before(started) {
return nil, nil
}
- minutes := uint(resolved.Sub(started).Minutes())
- resolutionDate := *resolved
+ minutes := uint(effective.Sub(started).Minutes())
+ resolutionDate := *effective
return &minutes, &resolutionDate
}
diff --git a/backend/plugins/rootly/tasks/incidents_converter_test.go
b/backend/plugins/rootly/tasks/incidents_converter_test.go
index 6ea99bd2d..422f719eb 100644
--- a/backend/plugins/rootly/tasks/incidents_converter_test.go
+++ b/backend/plugins/rootly/tasks/incidents_converter_test.go
@@ -85,7 +85,8 @@ func TestMapSeverityToPriority(t *testing.T) {
func TestComputeLeadTime_Resolved(t *testing.T) {
started := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
resolved := time.Date(2026, 5, 10, 11, 30, 0, 0, time.UTC)
- leadTime, resolutionDate := computeLeadTime(started, &resolved)
+ updated := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC)
+ leadTime, resolutionDate := computeLeadTime(started, &resolved,
updated, "resolved")
require.NotNil(t, leadTime)
require.NotNil(t, resolutionDate)
assert.Equal(t, uint(90), *leadTime)
@@ -94,15 +95,26 @@ func TestComputeLeadTime_Resolved(t *testing.T) {
func TestComputeLeadTime_Unresolved(t *testing.T) {
started := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
- leadTime, resolutionDate := computeLeadTime(started, nil)
+ updated := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC)
+ leadTime, resolutionDate := computeLeadTime(started, nil, updated,
"started")
assert.Nil(t, leadTime)
assert.Nil(t, resolutionDate)
}
+func TestComputeLeadTime_CompletedFallsBackToUpdated(t *testing.T) {
+ started := time.Date(2026, 3, 27, 20, 0, 0, 0, time.UTC)
+ updated := time.Date(2026, 3, 31, 14, 0, 0, 0, time.UTC)
+ leadTime, resolutionDate := computeLeadTime(started, nil, updated,
"completed")
+ require.NotNil(t, leadTime)
+ require.NotNil(t, resolutionDate)
+ assert.Equal(t, updated, *resolutionDate)
+}
+
func TestComputeLeadTime_ZeroDuration(t *testing.T) {
started := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
resolved := started
- leadTime, resolutionDate := computeLeadTime(started, &resolved)
+ updated := started
+ leadTime, resolutionDate := computeLeadTime(started, &resolved,
updated, "resolved")
require.NotNil(t, leadTime)
require.NotNil(t, resolutionDate)
assert.Equal(t, uint(0), *leadTime)
@@ -111,7 +123,8 @@ func TestComputeLeadTime_ZeroDuration(t *testing.T) {
func TestComputeLeadTime_ResolvedBeforeStarted(t *testing.T) {
started := time.Date(2026, 5, 10, 11, 0, 0, 0, time.UTC)
resolved := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
- leadTime, resolutionDate := computeLeadTime(started, &resolved)
+ updated := started
+ leadTime, resolutionDate := computeLeadTime(started, &resolved,
updated, "resolved")
assert.Nil(t, leadTime)
assert.Nil(t, resolutionDate)
}
diff --git a/backend/plugins/rootly/tasks/incidents_extractor.go
b/backend/plugins/rootly/tasks/incidents_extractor.go
index 99aca0946..3e812e2c7 100644
--- a/backend/plugins/rootly/tasks/incidents_extractor.go
+++ b/backend/plugins/rootly/tasks/incidents_extractor.go
@@ -64,9 +64,12 @@ func extractRootlyIncident(rawData []byte, op
*RootlyOptions) ([]interface{}, er
// Safety net: filter[service_ids] in the collector is the primary
// scope filter, but a regression there would let multi-service
- // incidents leak into a wrong scope's tool table.
- if services := rawIncident.Relationships.Services.Data; len(services) >
0 && !containsServiceId(services, op.ServiceId) {
- return nil, nil
+ // incidents leak into a wrong scope's tool table. When ServiceId is
+ // empty we are collecting all incidents globally, so skip this check.
+ if op.ServiceId != "" {
+ if services := rawIncident.Relationships.Services.Data;
len(services) > 0 && !containsServiceId(services, op.ServiceId) {
+ return nil, nil
+ }
}
if rawIncident.Attributes.StartedAt.IsZero() {
diff --git a/backend/plugins/rootly/tasks/task_data.go
b/backend/plugins/rootly/tasks/task_data.go
index 334483bbe..c530d1d17 100644
--- a/backend/plugins/rootly/tasks/task_data.go
+++ b/backend/plugins/rootly/tasks/task_data.go
@@ -37,9 +37,13 @@ type RootlyTaskData struct {
}
func (p *RootlyOptions) GetParams() any {
+ scopeId := p.ServiceId
+ if scopeId == "" {
+ scopeId = "all"
+ }
return models.RootlyParams{
ConnectionId: p.ConnectionId,
- ScopeId: p.ServiceId,
+ ScopeId: scopeId,
}
}
@@ -65,9 +69,6 @@ func DecodeTaskOptions(options map[string]interface{})
(*RootlyOptions, errors.E
}
func ValidateTaskOptions(op *RootlyOptions) errors.Error {
- if op.ServiceId == "" {
- return errors.BadInput.New("not enough info for Rootly
execution")
- }
if op.ConnectionId == 0 {
return errors.BadInput.New("connectionId is invalid")
}
