Include error message in traces when transport fails (Azure#21800)

Unwrap *url.Error types to avoid disclosure of unsanitized URLs.
Skip generating trace info for no-op tracers.

Author: jhendrixMSFT

sdk/azcore/CHANGELOG.md

@@ -9,9 +9,12 @@
 ### Bugs Fixed
 
 * Fixed an issue that could cause some allowed HTTP header values to not show up in logs.
+* Include error text instead of error type in traces when the transport returns an error.
 
 ### Other Changes
 
+* Skip generating trace info for no-op tracers.
+
 ## 1.9.0-beta.1 (2023-10-05)
 
 ### Other Changes

sdk/azcore/arm/runtime/policy_trace_namespace.go

@@ -18,7 +18,7 @@ import (
 // httpTraceNamespacePolicy is a policy that adds the az.namespace attribute to the current Span
 func httpTraceNamespacePolicy(req *policy.Request) (resp *http.Response, err error) {
 	rawTracer := req.Raw().Context().Value(shared.CtxWithTracingTracer{})
-	if tracer, ok := rawTracer.(tracing.Tracer); ok {
+	if tracer, ok := rawTracer.(tracing.Tracer); ok && tracer.Enabled() {
 		rt, err := resource.ParseResourceType(req.Raw().URL.Path)
 		if err == nil {
 			// add the namespace attribute to the current span

sdk/azcore/runtime/policy_http_trace.go

@@ -8,8 +8,10 @@ package runtime
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore/internal/exported"
@@ -44,7 +46,7 @@ type httpTracePolicy struct {
 // Do implements the pipeline.Policy interfaces for the httpTracePolicy type.
 func (h *httpTracePolicy) Do(req *policy.Request) (resp *http.Response, err error) {
 	rawTracer := req.Raw().Context().Value(shared.CtxWithTracingTracer{})
-	if tracer, ok := rawTracer.(tracing.Tracer); ok {
+	if tracer, ok := rawTracer.(tracing.Tracer); ok && tracer.Enabled() {
 		attributes := []tracing.Attribute{
 			{Key: attrHTTPMethod, Value: req.Raw().Method},
 			{Key: attrHTTPURL, Value: getSanitizedURL(*req.Raw().URL, h.allowedQP)},
@@ -74,9 +76,14 @@ func (h *httpTracePolicy) Do(req *policy.Request) (resp *http.Response, err erro
 					span.SetAttributes(tracing.Attribute{Key: attrAZServiceReqID, Value: reqID})
 				}
 			} else if err != nil {
-				// including the output from err.Error() might disclose URL query parameters.
-				// so instead of attempting to sanitize the output, we simply output the error type.
-				span.SetStatus(tracing.SpanStatusError, fmt.Sprintf("%T", err))
+				var urlErr *url.Error
+				if errors.As(err, &urlErr) {
+					// calling *url.Error.Error() will include the unsanitized URL
+					// which we don't want. in addition, we already have the HTTP verb
+					// and sanitized URL in the trace so we aren't losing any info
+					err = urlErr.Err
+				}
+				span.SetStatus(tracing.SpanStatusError, err.Error())
 			}
 			span.End()
 		}()

sdk/azcore/runtime/policy_http_trace_test.go

@@ -8,6 +8,7 @@ package runtime
 
 import (
 	"context"
+	"errors"
 	"io"
 	"net"
 	"net/http"
@@ -100,7 +101,22 @@ func TestHTTPTracePolicy(t *testing.T) {
 	require.Error(t, err)
 	require.ErrorIs(t, err, net.ErrClosed)
 	require.EqualValues(t, tracing.SpanStatusError, spanStatus)
-	require.EqualValues(t, "poll.errNetClosing", spanStatusStr)
+	require.EqualValues(t, "use of closed network connection", spanStatusStr)
+
+	const urlErrText = "the endpoint is invalid"
+	req, err = exported.NewRequest(context.WithValue(context.Background(), shared.CtxWithTracingTracer{}, tr), http.MethodGet, srv.URL())
+	require.NoError(t, err)
+	srv.AppendError(&url.Error{
+		Op:  http.MethodGet,
+		URL: srv.URL(),
+		Err: errors.New(urlErrText),
+	})
+	_, err = pl.Do(req)
+	require.Error(t, err)
+	var urlErr *url.Error
+	require.False(t, errors.As(err, &urlErr))
+	require.EqualValues(t, tracing.SpanStatusError, spanStatus)
+	require.EqualValues(t, urlErrText, spanStatusStr)
 }
 
 func TestStartSpan(t *testing.T) {