feat(trace): add concurrent-safe `Reset` method to `SpanRecorder`

[flc1125]

We probably need a Reset method to reuse it for testing. Just like InMemoryExporter.

https://github.com/open-telemetry/opentelemetry-go/blob/b99d2b81783dd3d27201393fc0e741a6fb4a8d6b/sdk/trace/tracetest/exporter.go#L61-L65

ex:

package main

import (
    "context"
    "testing"

    "github.com/stretchr/testify/assert"

    "go.opentelemetry.io/otel/sdk/trace"
    "go.opentelemetry.io/otel/sdk/trace/tracetest"
)

func TestReset(t *testing.T) {
    sr := tracetest.NewSpanRecorder()
    tp := trace.NewTracerProvider(trace.WithSpanProcessor(sr))
    ctx := context.Background()
    tracer := tp.Tracer("test")

    // test one:
    _, span := tracer.Start(ctx, "one")
    span.End()

    assert.Len(t, sr.Ended(), 1)
    t.Logf("the one span length: %d", len(sr.Ended()))

    // test two:
    sr.Reset() // <=== I don't want the result to be affected by one.
    _, span = tracer.Start(ctx, "two")
    span.End()

    assert.Len(t, sr.Ended(), 1) // <=== I don't want the result to be affected by one.
    t.Logf("the two span length: %d", len(sr.Ended()))
}

output:

=== RUN   TestReset
    main_test.go:24: the one span length: 1
    main_test.go:32: the two span length: 1
--- PASS: TestReset (0.00s)
PASS

Because it is a tool used for testing, I think such operations may occur.

[MrAlias]

Can you provide a use-case for this? All of our uses have not needed this functionality.

[flc1125]

Can you provide a use-case for this? All of our uses have not needed this functionality.

I have already added it to the description.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 82.1%. Comparing base (e98ef1b) to head (1c7d242). Report is 1 commits behind head on main.

Additional details and impacted files

[](https://app.codecov.io/gh/open-telemetry/opentelemetry-go/pull/5994?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) ```diff @@ Coverage Diff @@ ## main #5994 +/- ## ===================================== Coverage 82.1% 82.1% ===================================== Files 273 273 Lines 23616 23624 +8 ===================================== + Hits 19399 19406 +7 - Misses 3872 3873 +1 Partials 345 345 ``` [see 3 files with indirect coverage changes](https://app.codecov.io/gh/open-telemetry/opentelemetry-go/pull/5994/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)

[pellared]

In the same package, InMemoryExporter has a Reset method. So applying the same to SpanRecorder sounds sensible.

We have also a Reset here: https://pkg.go.dev/go.opentelemetry.io/otel/log/logtest#Recorder.Reset

[MrAlias]

We probably need a Reset method to reuse it for testing.

I'm not in favor of adding additional surface area to the API based on possibilities. I would like there to be valid use cases instead of arguments about symmetry and hypothetical that motivate us.

[flc1125]

We probably need a Reset method to reuse it for testing.

I'm not in favor of adding additional surface area to the API based on possibilities. I would like there to be valid use cases instead of arguments about symmetry and hypothetical that motivate us.

I'll mention the background for adding this feature:

When I was writing the unit tests for otelgin, initially I wanted to implement logic similar to the following:

package main

import (
    "testing"

    "go.opentelemetry.io/otel"
    "go.opentelemetry.io/otel/sdk/trace"
    "go.opentelemetry.io/otel/sdk/trace/tracetest"
)

func TestOtel(t *testing.T) {
    imsb := tracetest.NewInMemoryExporter()
    otel.SetTracerProvider(trace.NewTracerProvider(
        trace.WithSyncer(imsb),
    ))

    t.Run("test1", func(t *testing.T) {
        defer imsb.Reset()

        // do something...

        // check imsb...
    })

    t.Run("test2", func(t *testing.T) {
        defer imsb.Reset()

        // do something...

        // check imsb...
    })
}

Because, I hope that imsb and otel.SetTracerProvider only need to be set once for convenience in subsequent repeated use.

However, I found that a large amount of test logic uses SpanRecorder, based on the "herd mentality", I also adjusted to use SpanRecorder. But I discovered that it does not support the Reset() method.

Therefore, this PR came into being.

---

I think the tracetest package should be used for unit testing, since InMemoryExporter has it, and logtest has it too. I feel that SpanExporter could also have it.

[MrAlias]

When I was writing the unit tests for otelgin, initially I wanted to implement logic similar to the following:

package main

import (
  "testing"

  "go.opentelemetry.io/otel"
  "go.opentelemetry.io/otel/sdk/trace"
  "go.opentelemetry.io/otel/sdk/trace/tracetest"
)

func TestOtel(t *testing.T) {
  imsb := tracetest.NewInMemoryExporter()
  otel.SetTracerProvider(trace.NewTracerProvider(
      trace.WithSyncer(imsb),
  ))

  t.Run("test1", func(t *testing.T) {
      defer imsb.Reset()

      // do something...

      // check imsb...
  })

  t.Run("test2", func(t *testing.T) {
      defer imsb.Reset()

      // do something...

      // check imsb...
  })
}

Thanks for the use-case.

I would recommend not using the global in testing, but that is a review for another PR. :wink: