Populate Request.values on creation (Azure#24817)

* Populate Request.values on creation

This ensures that the same instance is propagated across policies and
that any values are retained across retries.

* improve test

Author: jhendrixMSFT

sdk/azcore/CHANGELOG.md

@@ -8,6 +8,8 @@
 
 ### Bugs Fixed
 
+* Fixed incorrect request/response logging try info when logging a request that's being retried.
+
 ### Other Changes
 
 ## 1.18.0 (2025-04-03)

sdk/azcore/internal/exported/request.go

@@ -71,7 +71,8 @@ func (ov opValues) get(value any) bool {
 // NewRequestFromRequest creates a new policy.Request with an existing *http.Request
 // Exported as runtime.NewRequestFromRequest().
 func NewRequestFromRequest(req *http.Request) (*Request, error) {
-	policyReq := &Request{req: req}
+	// populate values so that the same instance is propagated across policies
+	policyReq := &Request{req: req, values: opValues{}}
 
 	if req.Body != nil {
 		// we can avoid a body copy here if the underlying stream is already a
@@ -117,7 +118,8 @@ func NewRequest(ctx context.Context, httpMethod string, endpoint string) (*Reque
 	if !(req.URL.Scheme == "http" || req.URL.Scheme == "https") {
 		return nil, fmt.Errorf("unsupported protocol scheme %s", req.URL.Scheme)
 	}
-	return &Request{req: req}, nil
+	// populate values so that the same instance is propagated across policies
+	return &Request{req: req, values: opValues{}}, nil
 }
 
 // Body returns the original body specified when the Request was created.

sdk/azcore/runtime/policy_retry_test.go

@@ -10,9 +10,11 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"math"
 	"net/http"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -962,3 +964,49 @@ func TestCalcDelay(t *testing.T) {
 		}
 	})
 }
+
+type retryLoggingChecker struct {
+	count int32
+}
+
+func (r *retryLoggingChecker) Do(req *policy.Request) (*http.Response, error) {
+	if r.count > 0 {
+		var opValues logPolicyOpValues
+		req.OperationValue(&opValues)
+		if reflect.ValueOf(opValues).IsZero() {
+			// the logging policy is after us. so it should have populated logPolicyOpValues
+			return nil, errors.New("unexpected zero-value for logPolicyOpValues")
+		}
+
+		// verify that the logging policy is updating the try in opValues
+		if r.count != opValues.try {
+			return nil, fmt.Errorf("expected count %d, got %d", r.count, opValues.try)
+		}
+	}
+	r.count++
+	return req.Next()
+}
+
+func TestRetryPolicyWithLoggingChecker(t *testing.T) {
+	srv, close := mock.NewServer()
+	defer close()
+	srv.AppendResponse(mock.WithBodyReadError())
+	srv.AppendResponse(mock.WithBodyReadError())
+	srv.AppendResponse()
+	pl := newTestPipeline(&policy.ClientOptions{
+		Transport: srv, Retry: *testRetryOptions(),
+		PerRetryPolicies: []policy.Policy{&retryLoggingChecker{}},
+	})
+	req, err := NewRequest(context.Background(), http.MethodGet, srv.URL())
+	require.NoError(t, err)
+
+	body := newRewindTrackingBody("stuff")
+	require.NoError(t, req.SetBody(body, "text/plain"))
+
+	resp, err := pl.Do(req)
+	require.NoError(t, err)
+	require.EqualValues(t, http.StatusOK, resp.StatusCode)
+	require.EqualValues(t, 3, srv.Requests())
+	require.EqualValues(t, 2, body.rcount)
+	require.True(t, body.closed)
+}