Handle HTTP 304s when setting span status

interceptor.go

@@ -159,7 +159,7 @@ func (i *Interceptor) WrapUnary(next connect.UnaryFunc) connect.UnaryFunc {
 			),
 		)
 		attributes = attributeFilter(req, attributes...)
-		span.SetStatus(spanStatus(err))
+		span.SetStatus(spanStatus(protocol, err))
 		span.SetAttributes(attributes...)
 		instrumentation.duration.Record(ctx, i.config.now().Sub(requestStartTime).Milliseconds(), metric.WithAttributes(attributes...))
 		instrumentation.requestSize.Record(ctx, int64(requestSize), metric.WithAttributes(attributes...))
@@ -234,7 +234,7 @@ func (i *Interceptor) WrapStreamingClient(next connect.StreamingClientFunc) conn
 				}
 				span.SetAttributes(state.attributes...)
 				span.SetAttributes(headerAttributes(protocol, responseKey, conn.ResponseHeader(), i.config.responseHeaderKeys)...)
-				span.SetStatus(spanStatus(state.error))
+				span.SetStatus(spanStatus(protocol, state.error))
 				span.End()
 				instrumentation.duration.Record(ctx, i.config.now().Sub(requestStartTime).Milliseconds(), metric.WithAttributes(state.attributes...))
 			},
@@ -317,7 +317,7 @@ func (i *Interceptor) WrapStreamingHandler(next connect.StreamingHandlerFunc) co
 		}
 		span.SetAttributes(state.attributes...)
 		span.SetAttributes(headerAttributes(protocol, responseKey, conn.ResponseHeader(), i.config.responseHeaderKeys)...)
-		span.SetStatus(spanStatus(err))
+		span.SetStatus(spanStatus(protocol, err))
 		instrumentation.duration.Record(ctx, i.config.now().Sub(requestStartTime).Milliseconds(), metric.WithAttributes(state.attributes...))
 		return err
 	}
@@ -336,10 +336,13 @@ func protocolToSemConv(protocol string) string {
 	}
 }
 
-func spanStatus(err error) (codes.Code, string) {
+func spanStatus(protocol string, err error) (codes.Code, string) {
 	if err == nil {
 		return codes.Unset, ""
 	}
+	if protocol == connectProtocol && connect.IsNotModifiedError(err) {
+		return codes.Unset, ""
+	}
 	if connectErr := new(connect.Error); errors.As(err, &connectErr) {
 		return codes.Error, connectErr.Message()
 	}

interceptor_test.go

@@ -1434,6 +1434,51 @@ func TestUnaryInterceptorPropagation(t *testing.T) {
 	assert.True(t, recordedSpan.Parent().Equal(span.SpanContext()))
 }
 
+func TestUnaryInterceptorNotModifiedError(t *testing.T) {
+	t.Parallel()
+	spanRecorder := tracetest.NewSpanRecorder()
+	traceProvider := trace.NewTracerProvider(trace.WithSpanProcessor(spanRecorder))
+	var ctx context.Context
+	client, _, _ := startServer(
+		[]connect.HandlerOption{
+			connect.WithInterceptors(connect.UnaryInterceptorFunc(func(unaryFunc connect.UnaryFunc) connect.UnaryFunc {
+				return func(_ context.Context, request connect.AnyRequest) (connect.AnyResponse, error) {
+					ctx, _ = trace.NewTracerProvider().Tracer("test").Start(context.Background(), "test")
+					return unaryFunc(ctx, request)
+				}
+			})),
+			connect.WithInterceptors(NewInterceptor(
+				WithPropagator(propagation.TraceContext{}),
+				WithTracerProvider(traceProvider),
+				WithTrustRemote(),
+			)),
+		},
+		[]connect.ClientOption{
+			connect.WithHTTPGet(),
+		},
+		happyPingServer(),
+	)
+	req := connect.NewRequest(&pingv1.PingRequest{Id: 1})
+	req.Header().Set("If-None-Match", cacheablePingEtag)
+	_, err := client.Ping(context.Background(), req)
+	assert.ErrorContains(t, err, "not modified")
+	assert.True(t, connect.IsNotModifiedError(err))
+	assert.Equal(t, len(spanRecorder.Ended()), 1)
+	recordedSpan := spanRecorder.Ended()[0]
+	assert.Equal(t, codes.Unset, recordedSpan.Status().Code)
+	var codeAttributes []attribute.KeyValue
+	for _, attr := range recordedSpan.Attributes() {
+		if attr.Key == semconv.HTTPStatusCodeKey {
+			codeAttributes = append(codeAttributes, attr)
+		} else if strings.HasPrefix(string(attr.Key), "rpc") && strings.HasSuffix(string(attr.Key), "code") {
+			codeAttributes = append(codeAttributes, attr)
+		}
+	}
+	// should not be any RPC status attribute, only the HTTP status attribute
+	expectedCodeAttributes := []attribute.KeyValue{semconv.HTTPStatusCodeKey.Int(304)}
+	assert.Equal(t, expectedCodeAttributes, codeAttributes)
+}
+
 func TestWithUntrustedRemoteUnary(t *testing.T) {
 	t.Parallel()
 	var propagator propagation.TraceContext

internal/proto/observability/ping/v1/ping.proto

@@ -58,7 +58,9 @@ message CumSumResponse {
 
 service PingService {
   // Ping sends a ping to the server to determine if it's reachable.
-  rpc Ping(PingRequest) returns (PingResponse) {}
+  rpc Ping(PingRequest) returns (PingResponse) {
+    option idempotency_level = NO_SIDE_EFFECTS;
+  }
   // Fail always fails.
   rpc Fail(FailRequest) returns (FailResponse) {}
   // Sum calculates the sum of the numbers sent on the stream.

pingserver_test.go

@@ -19,12 +19,15 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net/http"
 
 	"github.com/bufbuild/connect-go"
 	pingv1 "github.com/bufbuild/connect-opentelemetry-go/internal/gen/observability/ping/v1"
 	"github.com/bufbuild/connect-opentelemetry-go/internal/gen/observability/ping/v1/pingv1connect"
 )
 
+const cacheablePingEtag = "ABCDEFGH"
+
 func pingHappy(_ context.Context, req *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error) {
 	return connect.NewResponse(&pingv1.PingResponse{
 		Id:   req.Msg.Id,
@@ -103,7 +106,15 @@ func (p *pluggablePingServer) Ping(
 	ctx context.Context,
 	request *connect.Request[pingv1.PingRequest],
 ) (*connect.Response[pingv1.PingResponse], error) {
-	return p.ping(ctx, request)
+	if request.HTTPMethod() == http.MethodGet && request.Header().Get("If-None-Match") == cacheablePingEtag {
+		return nil, connect.NewNotModifiedError(nil)
+	}
+	resp, err := p.ping(ctx, request)
+	if err != nil {
+		return nil, err
+	}
+	resp.Header().Set("Etag", cacheablePingEtag)
+	return resp, nil
 }
 
 func (p *pluggablePingServer) CumSum(