Bugfix: When there are child spans, the duration of the parent_span i…

…s calculated incorrectly (#1243) (#1244)

* Bugfix: When there are child spans, the duration of the parent_span is calculated incorrectly

* Add Copyright for math package

* Fix gofmt

Co-authored-by: liuhaoyang <[email protected]>

modules/msp/apm/trace/trace.service.go

@@ -36,6 +36,7 @@ import (
 	"github.com/erda-project/erda/modules/msp/apm/trace/db"
 	"github.com/erda-project/erda/pkg/common/apis"
 	"github.com/erda-project/erda/pkg/common/errors"
+	mathpkg "github.com/erda-project/erda/pkg/math"
 )
 
 type traceService struct {
@@ -110,28 +111,37 @@ func (s *traceService) GetSpans(ctx context.Context, req *pb.GetSpansRequest) (*
 type void struct{}
 
 func (s *traceService) handleSpanResponse(spanTree SpanTree) (*pb.GetSpansResponse, error) {
-	var spans []*pb.Span
+	var (
+		spans          []*pb.Span
+		traceStartTime int64 = 0
+		traceEndTime   int64 = 0
+		depth          int64 = 0
+	)
+
 	spanCount := int64(len(spanTree))
-	durationTotal := int64(0)
-	depth := int64(0)
 	if spanCount > 0 {
 		depth = 1
 	}
 	services := map[string]void{}
-	for id, span := range spanTree {
+	for _, span := range spanTree {
 		services[span.Tags["service_name"]] = void{}
 		tempDepth := calculateDepth(depth, span, spanTree)
 		if tempDepth > depth {
 			depth = tempDepth
 		}
-		span.Duration = (span.EndTime - span.StartTime) - childSpanDuration(id, spanTree)
-		durationTotal += span.Duration
+		if traceStartTime == 0 || traceStartTime > span.StartTime {
+			traceStartTime = span.StartTime
+		}
+		if traceEndTime == 0 || traceEndTime < span.EndTime {
+			traceEndTime = span.EndTime
+		}
+		span.Duration = mathpkg.AbsInt64(span.EndTime - span.StartTime)
 		spans = append(spans, span)
 	}
 
 	serviceCount := int64(len(services))
 
-	return &pb.GetSpansResponse{Spans: spans, ServiceCount: serviceCount, Depth: depth, Duration: durationTotal, SpanCount: spanCount}, nil
+	return &pb.GetSpansResponse{Spans: spans, ServiceCount: serviceCount, Depth: depth, Duration: mathpkg.AbsInt64(traceEndTime - traceStartTime), SpanCount: spanCount}, nil
 }
 
 func calculateDepth(depth int64, span *pb.Span, spanTree SpanTree) int64 {
@@ -142,16 +152,6 @@ func calculateDepth(depth int64, span *pb.Span, spanTree SpanTree) int64 {
 	return depth
 }
 
-func childSpanDuration(id string, spanTree SpanTree) int64 {
-	duration := int64(0)
-	for _, span := range spanTree {
-		if span.ParentSpanId == id {
-			duration += span.EndTime - span.StartTime
-		}
-	}
-	return duration
-}
-
 func (s *traceService) GetSpanCount(ctx context.Context, traceID string) (int64, error) {
 	count := 0
 	s.p.cassandraSession.Query("SELECT COUNT(trace_id) FROM spans WHERE trace_id = ?", traceID).Iter().Scan(&count)

pkg/math/math.go

@@ -0,0 +1,29 @@
+// Copyright (c) 2021 Terminus, Inc.
+//
+// This program is free software: you can use, redistribute, and/or modify
+// it under the terms of the GNU Affero General Public License, version 3
+// or later ("AGPL"), as published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+package math
+
+func AbsInt(x int) int {
+	y := x >> 31
+	return (x ^ y) - y
+}
+
+func AbsInt32(x int32) int32 {
+	y := x >> 31
+	return (x ^ y) - y
+}
+
+func AbsInt64(x int64) int64 {
+	y := x >> 63
+	return (x ^ y) - y
+}

pkg/math/math_test.go

@@ -0,0 +1,52 @@
+// Copyright (c) 2021 Terminus, Inc.
+//
+// This program is free software: you can use, redistribute, and/or modify
+// it under the terms of the GNU Affero General Public License, version 3
+// or later ("AGPL"), as published by the Free Software Foundation.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+package math
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestAbsInt(t *testing.T) {
+	x := AbsInt(10)
+	assert.Equal(t, 10, x)
+
+	x = AbsInt(0)
+	assert.Equal(t, 0, x)
+
+	x = AbsInt(-10)
+	assert.Equal(t, 10, x)
+}
+
+func TestAbsInt32(t *testing.T) {
+	x := AbsInt32(10)
+	assert.Equal(t, int32(10), x)
+
+	x = AbsInt32(0)
+	assert.Equal(t, int32(0), x)
+
+	x = AbsInt32(-10)
+	assert.Equal(t, int32(10), x)
+}
+
+func TestAbsInt64(t *testing.T) {
+	x := AbsInt64(10)
+	assert.Equal(t, int64(10), x)
+
+	x = AbsInt64(0)
+	assert.Equal(t, int64(0), x)
+
+	x = AbsInt64(-10)
+	assert.Equal(t, int64(10), x)
+}