From b21ed4eb11f5036413b5f2c2b60b0eff0a186883 Mon Sep 17 00:00:00 2001 From: erda-bot <81558540+erda-bot@users.noreply.github.com> Date: Sat, 31 Jul 2021 20:03:09 +0800 Subject: [PATCH] Bugfix: When there are child spans, the duration of the parent_span is 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 --- modules/msp/apm/trace/trace.service.go | 34 ++++++++--------- pkg/math/math.go | 29 ++++++++++++++ pkg/math/math_test.go | 52 ++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 pkg/math/math.go create mode 100644 pkg/math/math_test.go diff --git a/modules/msp/apm/trace/trace.service.go b/modules/msp/apm/trace/trace.service.go index 11ce85cef91..0a117e6dcaf 100644 --- a/modules/msp/apm/trace/trace.service.go +++ b/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) diff --git a/pkg/math/math.go b/pkg/math/math.go new file mode 100644 index 00000000000..d3709644b8b --- /dev/null +++ b/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 . + +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 +} diff --git a/pkg/math/math_test.go b/pkg/math/math_test.go new file mode 100644 index 00000000000..b55f9e76032 --- /dev/null +++ b/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 . + +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) +}