From 803feb42bde04955700038a70c0a9a88b1e4349a Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 23:35:15 +0800 Subject: [PATCH 1/6] feat(trace): add concurrent-safe `Reset` method to `SpanRecorder` --- sdk/trace/tracetest/recorder.go | 12 ++++++++++++ sdk/trace/tracetest/recorder_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/sdk/trace/tracetest/recorder.go b/sdk/trace/tracetest/recorder.go index 7aababbbf2f..c326cc010eb 100644 --- a/sdk/trace/tracetest/recorder.go +++ b/sdk/trace/tracetest/recorder.go @@ -69,6 +69,18 @@ func (sr *SpanRecorder) Started() []sdktrace.ReadWriteSpan { return dst } +// Reset clears the recorded spans. +// +// This method is safe to be called concurrently. +func (sr *SpanRecorder) Reset() { + sr.startedMu.Lock() + defer sr.startedMu.Unlock() + sr.started = nil + sr.endedMu.Lock() + defer sr.endedMu.Unlock() + sr.ended = nil +} + // Ended returns a copy of all ended spans that have been recorded. // // This method is safe to be called concurrently. diff --git a/sdk/trace/tracetest/recorder_test.go b/sdk/trace/tracetest/recorder_test.go index 5fd2eecd11a..1d7c7723111 100644 --- a/sdk/trace/tracetest/recorder_test.go +++ b/sdk/trace/tracetest/recorder_test.go @@ -112,3 +112,27 @@ func TestStartingConcurrentSafe(t *testing.T) { assert.Len(t, sr.Started(), 2) } + +func TestResetConcurrentSafe(t *testing.T) { + sr := NewSpanRecorder() + ctx := context.Background() + + runConcurrently( + func() { sr.OnStart(ctx, new(rwSpan)) }, + func() { sr.OnStart(ctx, new(rwSpan)) }, + func() { sr.OnEnd(new(roSpan)) }, + func() { sr.OnEnd(new(roSpan)) }, + ) + + assert.Len(t, sr.Started(), 2) + assert.Len(t, sr.Ended(), 2) + + runConcurrently( + func() { sr.Reset() }, + func() { sr.Reset() }, + func() { sr.Reset() }, + ) + + assert.Empty(t, sr.Started()) + assert.Empty(t, sr.Ended()) +} From c6712254a22804c83176a34c40d4b4d85792a3a9 Mon Sep 17 00:00:00 2001 From: Flc Date: Thu, 21 Nov 2024 23:41:52 +0800 Subject: [PATCH 2/6] feat(trace): add concurrent-safe `Reset` method to `SpanRecorder` --- sdk/trace/tracetest/recorder.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/trace/tracetest/recorder.go b/sdk/trace/tracetest/recorder.go index c326cc010eb..15b46d5e7a6 100644 --- a/sdk/trace/tracetest/recorder.go +++ b/sdk/trace/tracetest/recorder.go @@ -74,8 +74,9 @@ func (sr *SpanRecorder) Started() []sdktrace.ReadWriteSpan { // This method is safe to be called concurrently. func (sr *SpanRecorder) Reset() { sr.startedMu.Lock() - defer sr.startedMu.Unlock() sr.started = nil + sr.startedMu.Unlock() + sr.endedMu.Lock() defer sr.endedMu.Unlock() sr.ended = nil From 17df698e98bd1da4536a5c0a4eba57b4ce1f507a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Flc=E3=82=9B?= Date: Fri, 22 Nov 2024 17:36:34 +0800 Subject: [PATCH 3/6] Update sdk/trace/tracetest/recorder.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- sdk/trace/tracetest/recorder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/trace/tracetest/recorder.go b/sdk/trace/tracetest/recorder.go index 15b46d5e7a6..9ff82f0597d 100644 --- a/sdk/trace/tracetest/recorder.go +++ b/sdk/trace/tracetest/recorder.go @@ -78,8 +78,8 @@ func (sr *SpanRecorder) Reset() { sr.startedMu.Unlock() sr.endedMu.Lock() - defer sr.endedMu.Unlock() sr.ended = nil + r.endedMu.Unlock() } // Ended returns a copy of all ended spans that have been recorded. From 9510a4db858c56bd7acd2e75dc4b00972a341262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 22 Nov 2024 10:44:10 +0100 Subject: [PATCH 4/6] Update sdk/trace/tracetest/recorder.go --- sdk/trace/tracetest/recorder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/trace/tracetest/recorder.go b/sdk/trace/tracetest/recorder.go index 9ff82f0597d..5b76df2e3ba 100644 --- a/sdk/trace/tracetest/recorder.go +++ b/sdk/trace/tracetest/recorder.go @@ -79,7 +79,7 @@ func (sr *SpanRecorder) Reset() { sr.endedMu.Lock() sr.ended = nil - r.endedMu.Unlock() + sr.endedMu.Unlock() } // Ended returns a copy of all ended spans that have been recorded. From ea3d1ac3711813017a25e2c2551f6074db4a0ae5 Mon Sep 17 00:00:00 2001 From: Flc Date: Fri, 22 Nov 2024 17:44:51 +0800 Subject: [PATCH 5/6] docs(CHANGELOG): update for new `SpanRecorder` --- CHANGELOG.md | 4 ++++ sdk/trace/tracetest/recorder.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0dcd6e6c8d..a67f1482e57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Added + +- Add concurrent-safe `Reset` method to `SpanRecorder` in `go.opentelemetry.io/otel/sdk/trace/tracetest`. (#5994) + ### Changed - Propagate non-retryable error messages to client in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#5929) diff --git a/sdk/trace/tracetest/recorder.go b/sdk/trace/tracetest/recorder.go index 9ff82f0597d..5b76df2e3ba 100644 --- a/sdk/trace/tracetest/recorder.go +++ b/sdk/trace/tracetest/recorder.go @@ -79,7 +79,7 @@ func (sr *SpanRecorder) Reset() { sr.endedMu.Lock() sr.ended = nil - r.endedMu.Unlock() + sr.endedMu.Unlock() } // Ended returns a copy of all ended spans that have been recorded. From 7f07b60ab2a80b48a0789c711f3093d1f82adf5a Mon Sep 17 00:00:00 2001 From: Flc Date: Fri, 22 Nov 2024 18:42:27 +0800 Subject: [PATCH 6/6] chore(recorder): Simplify `Reset` method in `SpanRecorder` for better concurrency safety --- CHANGELOG.md | 2 +- sdk/trace/tracetest/recorder.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8571901b892..e21619fc364 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added -- Add concurrent-safe `Reset` method to `SpanRecorder` in `go.opentelemetry.io/otel/sdk/trace/tracetest`. (#5994) +- Add `Reset` method to `SpanRecorder` in `go.opentelemetry.io/otel/sdk/trace/tracetest`. (#5994) ### Changed diff --git a/sdk/trace/tracetest/recorder.go b/sdk/trace/tracetest/recorder.go index 5b76df2e3ba..732669a17ad 100644 --- a/sdk/trace/tracetest/recorder.go +++ b/sdk/trace/tracetest/recorder.go @@ -74,12 +74,12 @@ func (sr *SpanRecorder) Started() []sdktrace.ReadWriteSpan { // This method is safe to be called concurrently. func (sr *SpanRecorder) Reset() { sr.startedMu.Lock() - sr.started = nil - sr.startedMu.Unlock() - sr.endedMu.Lock() + defer sr.startedMu.Unlock() + defer sr.endedMu.Unlock() + + sr.started = nil sr.ended = nil - sr.endedMu.Unlock() } // Ended returns a copy of all ended spans that have been recorded.