Skip to content

otelgrpc: Stablize TestInterceptors#4535

Merged
pellared merged 2 commits intoopen-telemetry:mainfrom
pellared:stab-intercetors-test
Nov 8, 2023
Merged

otelgrpc: Stablize TestInterceptors#4535
pellared merged 2 commits intoopen-telemetry:mainfrom
pellared:stab-intercetors-test

Conversation

@pellared
Copy link
Copy Markdown
Member

@pellared pellared commented Nov 7, 2023

Fixes #1352

The problem is that StreamClientInterceptor ends the spans asynchronously:

stream := wrapClientStream(ctx, s, desc, cfg)
go func() {
err := <-stream.finished
if err != nil {
s, _ := status.FromError(err)
span.SetStatus(codes.Error, s.Message())
span.SetAttributes(statusCodeAttr(s.Code()))
} else {
span.SetAttributes(statusCodeAttr(grpc_codes.OK))
}
span.End()
}()

It may seem that it would be better to reimplement StreamClientInterceptor. However, I do not find it pragmatic given that we want to deprecate the interceptors anyway.

EDIT: I think I have implemented a fix for the StreamClientInterceptor here: #4537.

@pellared pellared added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label Nov 7, 2023
@pellared pellared marked this pull request as ready for review November 7, 2023 08:53
@pellared pellared requested a review from a team November 7, 2023 08:53
@pellared pellared merged commit f6aeb0d into open-telemetry:main Nov 8, 2023
@pellared pellared deleted the stab-intercetors-test branch November 8, 2023 14:56
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: TestInterceptors/StreamClientSpans

3 participants