Tracing: Various improvements (#88308)

contribute/backend/instrumentation.md

@@ -194,12 +194,11 @@ func (s *MyService) Hello(ctx context.Context, name string) (string, error) {
    if name == "" {
       err := fmt.Errorf("name cannot be empty")
 
-      // sets the span’s status to Error to make the span tracking
+      // Use the helper functions tracing.Errorf or tracing.Error
-      // a failed operation as an error span.
+      // to set the span’s status to Error to make
-      span.SetStatus(codes.Error, "failed to check name")
+      // the span tracking a failed operation as an error span and
-      // record err as an exception span event for this span
+      // record error as an exception span event for the provided span.
-      span.RecordError(err)
+      return "", tracing.Errorf(span, "failed to check name: %w", err)
-      return "", err
    }
 
    // Add some other event to show Events usage

pkg/infra/tracing/tracing.go

@@ -2,6 +2,7 @@ package tracing
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"math"
 	"net"
@@ -14,6 +15,7 @@ import (
 	"go.opentelemetry.io/contrib/samplers/jaegerremote"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
+	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/exporters/jaeger"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
@@ -27,6 +29,7 @@ import (
 	"github.com/go-kit/log/level"
 
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/util/errutil"
 )
 
 const (
@@ -109,6 +112,25 @@ func TraceIDFromContext(ctx context.Context, requireSampled bool) string {
 	return spanCtx.TraceID().String()
 }
 
+// Error sets the status to error and record the error as an exception in the provided span.
+func Error(span trace.Span, err error) error {
+	attr := []attribute.KeyValue{}
+	grafanaErr := errutil.Error{}
+	if errors.As(err, &grafanaErr) {
+		attr = append(attr, attribute.String("message_id", grafanaErr.MessageID))
+	}
+
+	span.SetStatus(codes.Error, err.Error())
+	span.RecordError(err, trace.WithAttributes(attr...))
+	return err
+}
+
+// Errorf wraps fmt.Errorf and also sets the status to error and record the error as an exception in the provided span.
+func Errorf(span trace.Span, format string, args ...any) error {
+	err := fmt.Errorf(format, args...)
+	return Error(span, err)
+}
+
 type noopTracerProvider struct {
 	trace.TracerProvider
 }

pkg/registry/apis/playlist/conversions.go

@@ -91,7 +91,7 @@ func convertToK8sResource(v *playlistsvc.PlaylistDTO, namespacer request.Namespa
 	if err == nil {
 		meta.SetUpdatedTimestampMillis(v.UpdatedAt)
 		if v.Id > 0 {
-			createdAt := time.UnixMilli(v.CreatedAt)
+			createdAt := time.UnixMilli(v.CreatedAt).UTC()
 			meta.SetOriginInfo(&utils.ResourceOriginInfo{
 				Name:      "SQL",
 				Key:       fmt.Sprintf("%d", v.Id),

pkg/services/encryption/service/helpers.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/infra/usagestats"
 	encryptionprovider "github.com/grafana/grafana/pkg/services/encryption/provider"
 	"github.com/grafana/grafana/pkg/setting"
@@ -17,7 +18,7 @@ func SetupTestService(tb testing.TB) *Service {
 	provider := encryptionprovider.ProvideEncryptionProvider()
 	settings := setting.NewCfg()
 
-	service, err := ProvideEncryptionService(provider, usMock, settings)
+	service, err := ProvideEncryptionService(tracing.InitializeTracerForTest(), provider, usMock, settings)
 	require.NoError(tb, err)
 
 	return service

pkg/services/encryption/service/service.go

@@ -7,7 +7,10 @@ import (
 	"errors"
 	"fmt"
 
+	"go.opentelemetry.io/otel/attribute"
+
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/infra/usagestats"
 	"github.com/grafana/grafana/pkg/services/encryption"
 	"github.com/grafana/grafana/pkg/setting"
@@ -24,7 +27,8 @@ const (
 // Service must not be used for encryption.
 // Use secrets.Service implementing envelope encryption instead.
 type Service struct {
-	log log.Logger
+	tracer tracing.Tracer
+	log    log.Logger
 
 	cfg          *setting.Cfg
 	usageMetrics usagestats.Service
@@ -34,12 +38,14 @@ type Service struct {
 }
 
 func ProvideEncryptionService(
+	tracer tracing.Tracer,
 	provider encryption.Provider,
 	usageMetrics usagestats.Service,
 	cfg *setting.Cfg,
 ) (*Service, error) {
 	s := &Service{
-		log: log.New("encryption"),
+		tracer: tracer,
+		log:    log.New("encryption"),
 
 		ciphers:   provider.ProvideCiphers(),
 		deciphers: provider.ProvideDeciphers(),
@@ -93,6 +99,9 @@ func (s *Service) registerUsageMetrics() {
 }
 
 func (s *Service) Decrypt(ctx context.Context, payload []byte, secret string) ([]byte, error) {
+	ctx, span := s.tracer.Start(ctx, "encryption.service.Decrypt")
+	defer span.End()
+
 	var err error
 	defer func() {
 		if err != nil {
@@ -115,6 +124,8 @@ func (s *Service) Decrypt(ctx context.Context, payload []byte, secret string) ([
 		return nil, err
 	}
 
+	span.SetAttributes(attribute.String("encryption.algorithm", algorithm))
+
 	var decrypted []byte
 	decrypted, err = decipher.Decrypt(ctx, toDecrypt, secret)
 
@@ -163,6 +174,9 @@ func (s *Service) deriveEncryptionAlgorithm(payload []byte) (string, []byte, err
 }
 
 func (s *Service) Encrypt(ctx context.Context, payload []byte, secret string) ([]byte, error) {
+	ctx, span := s.tracer.Start(ctx, "encryption.service.Encrypt")
+	defer span.End()
+
 	var err error
 	defer func() {
 		if err != nil {
@@ -179,6 +193,8 @@ func (s *Service) Encrypt(ctx context.Context, payload []byte, secret string) ([
 		return nil, err
 	}
 
+	span.SetAttributes(attribute.String("encryption.algorithm", algorithm))
+
 	var encrypted []byte
 	encrypted, err = cipher.Encrypt(ctx, payload, secret)
 

pkg/services/encryption/service/service_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/infra/usagestats"
 	"github.com/grafana/grafana/pkg/services/encryption"
 	"github.com/grafana/grafana/pkg/services/encryption/provider"
@@ -20,7 +21,7 @@ func Test_Service(t *testing.T) {
 	usageStats := &usagestats.UsageStatsMock{}
 	settings := setting.NewCfg()
 
-	svc, err := ProvideEncryptionService(encProvider, usageStats, settings)
+	svc, err := ProvideEncryptionService(tracing.InitializeTracerForTest(), encProvider, usageStats, settings)
 	require.NoError(t, err)
 
 	t.Run("decrypt empty payload should return error", func(t *testing.T) {
@@ -79,7 +80,7 @@ func Test_Service_MissingProvider(t *testing.T) {
 	usageStats := &usagestats.UsageStatsMock{}
 	settings := setting.NewCfg()
 
-	service, err := ProvideEncryptionService(encProvider, usageStats, settings)
+	service, err := ProvideEncryptionService(tracing.InitializeTracerForTest(), encProvider, usageStats, settings)
 	assert.Nil(t, service)
 	assert.Error(t, err)
 }

pkg/services/secrets/manager/helpers.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"gopkg.in/ini.v1"
 
+	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/infra/usagestats"
 	encryptionprovider "github.com/grafana/grafana/pkg/services/encryption/provider"
 	encryptionservice "github.com/grafana/grafana/pkg/services/encryption/service"
@@ -40,10 +41,11 @@ func setupTestService(tb testing.TB, store secrets.Store, features featuremgmt.F
 	encProvider := encryptionprovider.Provider{}
 	usageStats := &usagestats.UsageStatsMock{}
 
-	encryption, err := encryptionservice.ProvideEncryptionService(encProvider, usageStats, cfg)
+	encryption, err := encryptionservice.ProvideEncryptionService(tracing.InitializeTracerForTest(), encProvider, usageStats, cfg)
 	require.NoError(tb, err)
 
 	secretsService, err := ProvideSecretsService(
+		tracing.InitializeTracerForTest(),
 		store,
 		osskmsproviders.ProvideService(encryption, cfg, features),
 		encryption,

pkg/services/secrets/manager/manager.go

@@ -15,6 +15,7 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	"github.com/grafana/grafana/pkg/infra/log"
+	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/infra/usagestats"
 	"github.com/grafana/grafana/pkg/services/encryption"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -35,6 +36,7 @@ var (
 )
 
 type SecretsService struct {
+	tracer     tracing.Tracer
 	store      secrets.Store
 	enc        encryption.Internal
 	cfg        *setting.Cfg
@@ -54,6 +56,7 @@ type SecretsService struct {
 }
 
 func ProvideSecretsService(
+	tracer tracing.Tracer,
 	store secrets.Store,
 	kmsProvidersService kmsproviders.Service,
 	enc encryption.Internal,
@@ -68,6 +71,7 @@ func ProvideSecretsService(
 	))
 
 	s := &SecretsService{
+		tracer:              tracer,
 		store:               store,
 		enc:                 enc,
 		cfg:                 cfg,
@@ -158,6 +162,9 @@ func (s *SecretsService) encryptedWithEnvelopeEncryption(payload []byte) bool {
 var b64 = base64.RawStdEncoding
 
 func (s *SecretsService) Encrypt(ctx context.Context, payload []byte, opt secrets.EncryptionOptions) ([]byte, error) {
+	ctx, span := s.tracer.Start(ctx, "secretsService.Encrypt")
+	defer span.End()
+
 	// Use legacy encryption service if featuremgmt.FlagDisableEnvelopeEncryption toggle is on
 	if s.features.IsEnabled(ctx, featuremgmt.FlagDisableEnvelopeEncryption) {
 		return s.enc.Encrypt(ctx, payload, s.cfg.SecretKey)
@@ -313,6 +320,9 @@ func newRandomDataKey() ([]byte, error) {
 }
 
 func (s *SecretsService) Decrypt(ctx context.Context, payload []byte) ([]byte, error) {
+	ctx, span := s.tracer.Start(ctx, "secretsService.Decrypt")
+	defer span.End()
+
 	var err error
 	defer func() {
 		opsCounter.With(prometheus.Labels{

pkg/services/secrets/manager/manager_test.go

@@ -11,6 +11,7 @@ import (
 	"gopkg.in/ini.v1"
 
 	"github.com/grafana/grafana/pkg/infra/db"
+	"github.com/grafana/grafana/pkg/infra/tracing"
 	"github.com/grafana/grafana/pkg/infra/usagestats"
 	encryptionprovider "github.com/grafana/grafana/pkg/services/encryption/provider"
 	encryptionservice "github.com/grafana/grafana/pkg/services/encryption/service"
@@ -192,7 +193,7 @@ func TestSecretsService_UseCurrentProvider(t *testing.T) {
 		encProvider := encryptionprovider.Provider{}
 		usageStats := &usagestats.UsageStatsMock{}
 
-		encryptionService, err := encryptionservice.ProvideEncryptionService(encProvider, usageStats, cfg)
+		encryptionService, err := encryptionservice.ProvideEncryptionService(tracing.InitializeTracerForTest(), encProvider, usageStats, cfg)
 		require.NoError(t, err)
 
 		features := featuremgmt.WithFeatures()
@@ -201,6 +202,7 @@ func TestSecretsService_UseCurrentProvider(t *testing.T) {
 		secretStore := database.ProvideSecretsStore(testDB)
 
 		secretsService, err := ProvideSecretsService(
+			tracing.InitializeTracerForTest(),
 			secretStore,
 			&kms,
 			encryptionService,
@@ -219,6 +221,7 @@ func TestSecretsService_UseCurrentProvider(t *testing.T) {
 		// secret service tries to find a DEK in a cache first before calling provider's decrypt
 		// to bypass the cache, we set up one more secrets service to test decrypting
 		svcDecrypt, err := ProvideSecretsService(
+			tracing.InitializeTracerForTest(),
 			secretStore,
 			&kms,
 			encryptionService,

pkg/services/store/testdata/public_testdata.golden.jsonc

@@ -1,5 +1,5 @@
 //  🌟 This was machine generated.  Do not edit. 🌟
-//  
+//
 //  Frame[0] {
 //      "type": "directory-listing",
 //      "typeVersion": [
@@ -10,7 +10,7 @@
 //          "HasMore": false
 //      }
 //  }
-//  Name: 
+//  Name:
 //  Dimensions: 3 Fields by 3 Rows
 //  +----------------------------+----------------------+---------------+
 //  | Name: name                 | Name: mediaType      | Name: size    |
@@ -21,8 +21,8 @@
 //  | example-with-style.geojson | application/geo+json | 3332          |
 //  | usa-states.geojson         | application/geo+json | 89263         |
 //  +----------------------------+----------------------+---------------+
-//  
+//
-//  
+//
 //  🌟 This was machine generated.  Do not edit. 🌟
 {
   "status": 200,