This is an automated email from the ASF dual-hosted git repository.
liuhan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-go.git
The following commit(s) were added to refs/heads/main by this push:
new 5510aef Fix wrong tracing context when trace have been sampled (#161)
5510aef is described below
commit 5510aef8b575a2bd87e1d851fb6531e73c6f7d92
Author: mrproliu <[email protected]>
AuthorDate: Thu Jan 11 18:54:37 2024 +0800
Fix wrong tracing context when trace have been sampled (#161)
---
CHANGES.md | 1 +
plugins/core/span_noop.go | 26 +++++++++++++++++++++++++-
plugins/core/span_tracing.go | 5 ++++-
plugins/core/tracing.go | 11 +++++++----
plugins/core/tracing_test.go | 36 ++++++++++++++++++++++++++++++++++++
5 files changed, 73 insertions(+), 6 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 4e98f9c..b4c2a70 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -19,6 +19,7 @@ Release Notes.
* Fix SW_AGENT_REPORTER_GRPC_MAX_SEND_QUEUE not working on metricsSendCh &
logSendCh chans of gRPC reporter.
* Fix ParseVendorModule error for special case in vendor/modules.txt.
* Fix enhance method error when unknown parameter type.
+* Fix wrong tracing context when trace have been sampled.
#### Issues and PR
- All issues are
[here](https://github.com/apache/skywalking/milestone/197?closed=1)
diff --git a/plugins/core/span_noop.go b/plugins/core/span_noop.go
index 85cff00..4fc0beb 100644
--- a/plugins/core/span_noop.go
+++ b/plugins/core/span_noop.go
@@ -24,6 +24,20 @@ import (
const noopContextValue = "N/A"
type NoopSpan struct {
+ stackCount int
+}
+
+func newSnapshotNoopSpan() *NoopSpan {
+ // snapshot noop span is not a real span
+ return &NoopSpan{
+ stackCount: 0,
+ }
+}
+
+func newNoopSpan() *NoopSpan {
+ return &NoopSpan{
+ stackCount: 1,
+ }
}
func (*NoopSpan) GetTraceID() string {
@@ -75,7 +89,17 @@ func (*NoopSpan) Log(...string) {
func (*NoopSpan) Error(...string) {
}
-func (*NoopSpan) End() {
+func (n *NoopSpan) enterNoSpan() {
+ n.stackCount++
+}
+
+func (n *NoopSpan) End() {
+ n.stackCount--
+ if n.stackCount == 0 {
+ if ctx := getTracingContext(); ctx != nil {
+ ctx.SaveActiveSpan(nil)
+ }
+ }
}
func (*NoopSpan) IsEntry() bool {
diff --git a/plugins/core/span_tracing.go b/plugins/core/span_tracing.go
index 2368c7e..2871e37 100644
--- a/plugins/core/span_tracing.go
+++ b/plugins/core/span_tracing.go
@@ -368,10 +368,13 @@ func newSegmentRoot(segmentSpan *SegmentSpanImpl)
*RootSegmentSpan {
return s
}
-func newSnapshotSpan(current TracingSpan) *SnapshotSpan {
+func newSnapshotSpan(current TracingSpan) TracingSpan {
if current == nil {
return nil
}
+ if _, isNoop := current.(*NoopSpan); isNoop {
+ return newSnapshotNoopSpan()
+ }
segmentSpan, ok := current.(SegmentSpan)
if !ok || !segmentSpan.IsValid() { // is not segment span or segment is
invalid(Executed End() method
return nil
diff --git a/plugins/core/tracing.go b/plugins/core/tracing.go
index c3e493f..fb7b132 100644
--- a/plugins/core/tracing.go
+++ b/plugins/core/tracing.go
@@ -227,12 +227,16 @@ func (s *ContextSnapshot) IsValid() bool {
func (t *Tracer) createNoop() (*TracingContext, TracingSpan, bool) {
if !t.InitSuccess() || t.Reporter.ConnectionStatus() ==
reporter.ConnectionStatusDisconnect {
- return nil, &NoopSpan{}, true
+ return nil, newNoopSpan(), true
}
ctx := getTracingContext()
if ctx != nil {
span := ctx.ActiveSpan()
- _, ok := span.(*NoopSpan)
+ noop, ok := span.(*NoopSpan)
+ if ok {
+ // increase the stack count for ensure the noop span
can be clear in the context
+ noop.enterNoSpan()
+ }
return ctx, span, ok
}
ctx = NewTracingContext()
@@ -255,8 +259,7 @@ func (t *Tracer) createSpan0(ctx *TracingContext, parent
TracingSpan, pluginOpts
sampled := t.Sampler.IsSampled(ds.OperationName)
if !sampled {
// Filter by sample just return noop span
- s = &NoopSpan{}
- return s, nil
+ return newNoopSpan(), nil
}
}
// process the opts from agent core for prepare building segment span
diff --git a/plugins/core/tracing_test.go b/plugins/core/tracing_test.go
index 86b2573..a7b464c 100644
--- a/plugins/core/tracing_test.go
+++ b/plugins/core/tracing_test.go
@@ -419,6 +419,42 @@ func TestContext(t *testing.T) {
assert.Nil(t, tracing.ActiveSpan(), "active span should be nil")
}
+func TestNoopSpan(t *testing.T) {
+ defer ResetTracingContext()
+ Tracing.Sampler = NewConstSampler(false)
+ var err error
+ // create multiple noop span
+ span, err := tracing.CreateLocalSpan("test")
+ assert.Nil(t, err, "create span error")
+ assert.NotNil(t, span, "span should not be nil")
+ span1, err := tracing.CreateLocalSpan("test2")
+ assert.Nil(t, err, "create span error")
+ assert.NotNil(t, span, "span should not be nil")
+ activeSpan := tracing.ActiveSpan()
+ assert.NotNil(t, activeSpan, "active span should not be nil")
+ context := tracing.CaptureContext()
+ assert.NotNil(t, context, "context should not be nil")
+ oldGLS := GetGLS()
+
+ // switch to a new GLS(continue context and create span test)
+ SetAsNewGoroutine()
+ tracing.ContinueContext(context)
+ assert.NotNil(t, tracing.ActiveSpan(), "active span should not be nil")
+ test3span, err := tracing.CreateLocalSpan("test3")
+ assert.Nil(t, err, "create span error")
+ assert.NotNil(t, test3span, "span should not be nil")
+ test3span.End()
+ activeSpan = tracing.ActiveSpan()
+ assert.Nil(t, activeSpan, "active span should be nil")
+
+ // switch back to the GLS(check the active span should be nil, make
sure context is clean)
+ SetGLS(oldGLS)
+ span1.End()
+ span.End()
+ activeSpan = tracing.ActiveSpan()
+ assert.Nil(t, activeSpan, "active span should be nil")
+}
+
func validateSpanOperation(t *testing.T, cases []spanOperationTestCase) {
for _, tt := range cases {
spans := make([]tracing.Span, 0)
