Can tracetest.SpanRecorder be a concrete type?

[rakyll]

In the tracetest package, we have a SpanRecorder interface and a standard implementation named "StandardSpanRecorder".

I was thinking about use cases where users would need to reimplement this interface. If the user want flexibility and want to collect these events in a custom way, they already have access to OnStart and OnEnd hooks. Are we providing the SpanRecorder interface in case user want to rely on a different implementation that doesn't buffer or have as much contention as the StandardSpanRecorder? But then, given this is a utility for testing, I wonder if we are overabstracting the recorder.

Any comments would be appreciated.

[MrAlias]

Sorry about the delay. This sounds like a reasonable question and I think we can indeed make it a concrete type.

Are we providing the SpanRecorder interface in case user want to rely on a different implementation that doesn't buffer or have as much contention as the StandardSpanRecorder?

Yeah that was the idea. Additionally, there were some testing setups that used a map to capture completed span.

https://github.com/open-telemetry/opentelemetry-go-contrib/blob/6d5d814987af9ceb48e904b79ee03bd7d1867cc7/instrumentation/google.golang.org/grpc/interceptor_test.go#L39-L61

https://github.com/open-telemetry/opentelemetry-go-contrib/blob/6d5d814987af9ceb48e904b79ee03bd7d1867cc7/instrumentation/net/http/httptrace/clienttrace_test.go#L32-L35

But then, given this is a utility for testing, I wonder if we are overabstracting the recorder.

I think this is correct, we are likely over-abstracting. We could just build out the StandardSpanRecorder to support both behaviours.

[MrAlias]

Also, if we do make this change, it probably goes without saying, but we sound rename the StandardSpanRecorder to just be SpanRecorder.

[punya]

I'm working on this, but I just realized that tracetest is part of oteltest, which IMO shouldn't be released as v1.0 as part of the initial stable release because it has dependencies on metrics as well.

Can we remove the required-for-ga label @MrAlias @Aneurysm9 @rakyll ?

[rakyll]

@punya, this was under trace/tracetest when I filed the issue so you are right that it doesn't have to be stable at this point.