Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix wrong tracing context when trace have been sampled #161

Merged
merged 1 commit into from
Jan 11, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -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)
26 changes: 25 additions & 1 deletion plugins/core/span_noop.go
Original file line number Diff line number Diff line change
@@ -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 {
5 changes: 4 additions & 1 deletion plugins/core/span_tracing.go
Original file line number Diff line number Diff line change
@@ -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
11 changes: 7 additions & 4 deletions plugins/core/tracing.go
Original file line number Diff line number Diff line change
@@ -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
36 changes: 36 additions & 0 deletions plugins/core/tracing_test.go
Original file line number Diff line number Diff line change
@@ -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)