Revert "server: don't return trace recordings to tenants"

This reverts commit fc95ad8.

This PR restores the ability for tenant traces to see unstructured
recordings from the KV layer.

Please see note in cockroachdb#71694: cockroachdb#71694 (comment)

The need to enforce such strict redaction of traces for tenant queries
is not necessary. Customer views into the KV layer will be necessary in
order to debug query execution in multi-tenant settings. Redaction is
certainly needed at this boundary but must be configured more carefully.
Further PRs as necessitated by cockroachdb#58610 and cockroachdb#71694 will tackle these
issues on master (where trace redaction is still available) and will be
backported to the release branch as needed.

This PR restores tracing into KV for tenants and unblocks the ability
for serverless to use the release-21.2 branch.

Release note (ops change): Tracing queries from tenants will now include
unstructured log messages from the kv layer.

pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go

@@ -552,7 +552,6 @@ func TestOracle(t *testing.T) {
 // encountering this situation, and then follower reads work.
 func TestFollowerReadsWithStaleDescriptor(t *testing.T) {
  defer leaktest.AfterTest(t)()
- defer log.Scope(t).Close(t)
 
  ctx := context.Background()
  // The test uses follower_read_timestamp().

pkg/ccl/kvccl/kvtenantccl/tenant_trace_test.go

@@ -102,10 +102,7 @@ SET tracing = off;
  }
  }
  }
- // TODO(obs-inf): when traces are properly redactable, `visibleString` should make
- // it into the recording.
- // require.True(t, found, "trace for tenant missing trace message '%q':\n%s",
- // visibleString, sqlutils.MatrixToStr(results))
- _ = found
+ require.True(t, found, "trace for tenant missing trace message '%q':\n%s",
+ visibleString, sqlutils.MatrixToStr(results))
  })
 }

pkg/server/node.go

@@ -1083,26 +1083,12 @@ func (n *Node) setupSpanForIncomingRPC(
  return
  }
  if grpcSpan != nil {
- tenID, ok := roachpb.TenantFromContext(ctx)
  // If our local span descends from a parent on the other
  // end of the RPC (i.e. the !isLocalRequest) case,
  // attach the span recording to the batch response.
- //
- // However, don't do this for tenants, as trace data
- // may, in theory, contain sensitive information that
- // the tenant is not privy to.
- rec := grpcSpan.GetRecording()
- // This should be `if !ok || tenID != ...` but apparently not everyone
- // correctly populates the context. See TestFollowerReadsWithStaleDescriptor
- // for an example (and since this test uses the regular SQL connection, it
- // is likely a general problem).
- if ok && tenID != roachpb.SystemTenantID {
- for i := range rec {
- sp := &rec[i]
- sp.Tags, sp.Logs = nil, nil
- }
+ if rec := grpcSpan.GetRecording(); rec != nil {
+ br.CollectedSpans = append(br.CollectedSpans, rec...)
  }
- br.CollectedSpans = append(br.CollectedSpans, rec...)
  }
  }
  return ctx, finishSpan