Improve OTel/instrumentation setup (#18)

cmd/main.go

@@ -9,6 +9,8 @@ import (
 
 	// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
 	// to ensure that exec-entrypoint and run can make use of them.
+
+	"go.opentelemetry.io/otel"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
 	"k8s.io/apimachinery/pkg/runtime"
@@ -21,6 +23,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	resourcesv1alpha1 "github.com/altinn/altinn-k8s-operator/api/v1alpha1"
+	"github.com/altinn/altinn-k8s-operator/internal"
 	"github.com/altinn/altinn-k8s-operator/internal/controller"
 	"github.com/altinn/altinn-k8s-operator/internal/telemetry"
 	// +kubebuilder:scaffold:imports
@@ -65,10 +68,21 @@ func main() {
 	ctx := ctrl.SetupSignalHandler()
 
 	// Set up OpenTelemetry.
-	otelShutdown, err := telemetry.ConfigureOtel(ctx)
+	otelShutdown, err := telemetry.ConfigureOTel(ctx)
 	if err != nil {
-		return
+		setupLog.Error(err, "unable to configure OTel")
+		os.Exit(1)
 	}
+
+	ctx, span := otel.Tracer(telemetry.ServiceName).Start(ctx, "Main")
+
+	rt, err := internal.NewRuntime(ctx)
+	if err != nil {
+		setupLog.Error(err, "unable to initialize runtime")
+		span.End()
+		os.Exit(1)
+	}
+
 	// Handle shutdown properly so nothing leaks.
 	defer func() {
 		err = errors.Join(err, otelShutdown(context.Background()))
@@ -121,29 +135,35 @@ func main() {
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")
+		span.End()
 		os.Exit(1)
 	}
 
 	if err = (controller.NewMaskinportenClientReconciler(
-		ctx,
+		rt,
 		mgr.GetClient(),
 		mgr.GetScheme(),
 	)).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "MaskinportenClient")
+		span.End()
 		os.Exit(1)
 	}
 	// +kubebuilder:scaffold:builder
 
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up health check")
+		span.End()
 		os.Exit(1)
 	}
 	if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
 		setupLog.Error(err, "unable to set up ready check")
+		span.End()
 		os.Exit(1)
 	}
 
 	setupLog.Info("starting manager")
+	span.End()
+
 	if err := mgr.Start(ctx); err != nil {
 		setupLog.Error(err, "problem running manager")
 		os.Exit(1)

internal/config/azure_keyvault.go

@@ -10,6 +10,9 @@ import (
 )
 
 func loadFromAzureKeyVault(operatorContext *operatorcontext.Context) (*Config, error) {
+	span := operatorContext.StartSpan("GetConfig.AzureKeyVault")
+	defer span.End()
+
 	var cred azcore.TokenCredential
 	var err error
 
@@ -23,7 +26,7 @@ func loadFromAzureKeyVault(operatorContext *operatorcontext.Context) (*Config, e
 		return nil, fmt.Errorf("error getting credentials for loading config: %w", err)
 	}
 
-	url := fmt.Sprintf("https://altinn-%s-operator-kv.vault.azure.net", operatorContext.Env)
+	url := fmt.Sprintf("https://altinn-%s-operator-kv.vault.azure.net", operatorContext.Environment)
 	client, err := azsecrets.NewClient(url, cred, nil)
 	if err != nil {
 		return nil, fmt.Errorf("error building client for Azure KV: %w", err)
@@ -33,7 +36,7 @@ func loadFromAzureKeyVault(operatorContext *operatorcontext.Context) (*Config, e
 
 	config := &Config{}
 	for _, secretKey := range secretKeys {
-		secret, err := client.GetSecret(operatorContext, secretKey, "", nil)
+		secret, err := client.GetSecret(operatorContext.Context, secretKey, "", nil)
 		if err != nil {
 			return nil, fmt.Errorf("error getting secret: %s, %w", secretKey, err)
 		}

internal/config/config.go

@@ -17,6 +17,9 @@ type MaskinportenApiConfig struct {
 }
 
 func GetConfig(operatorContext *operatorcontext.Context, configFilePath string) (*Config, error) {
+	span := operatorContext.StartSpan("GetConfig")
+	defer span.End()
+
 	var cfg *Config
 	var err error
 	if operatorContext.IsLocal() {

internal/config/koanf.go

@@ -17,6 +17,9 @@ var (
 )
 
 func loadFromKoanf(operatorContext *operatorcontext.Context, configFilePath string) (*Config, error) {
+	span := operatorContext.StartSpan("GetConfig.Koanf")
+	defer span.End()
+
 	tryFindProjectRoot()
 
 	if configFilePath == "" {

internal/controller/maskinportenclient_controller.go

@@ -6,7 +6,6 @@ import (
 	"fmt"
 	"reflect"
 
-	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/codes"
 	"go.opentelemetry.io/otel/trace"
@@ -20,7 +19,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	resourcesv1alpha1 "github.com/altinn/altinn-k8s-operator/api/v1alpha1"
-	"github.com/altinn/altinn-k8s-operator/internal"
 	"github.com/altinn/altinn-k8s-operator/internal/maskinporten"
 	rt "github.com/altinn/altinn-k8s-operator/internal/runtime"
 )
@@ -33,24 +31,17 @@ type MaskinportenClientReconciler struct {
 	client.Client
 	Scheme  *runtime.Scheme
 	runtime rt.Runtime
-	tracer  trace.Tracer
 }
 
 func NewMaskinportenClientReconciler(
-	ctx context.Context,
+	rt rt.Runtime,
 	client client.Client,
 	scheme *runtime.Scheme,
 ) *MaskinportenClientReconciler {
-	rt, err := internal.NewRuntime(ctx)
-	if err != nil {
-		panic(err)
-	}
-
 	return &MaskinportenClientReconciler{
 		Client:  client,
 		Scheme:  scheme,
 		runtime: rt,
-		tracer:  otel.Tracer("MaskinportenClientReconciler"),
 	}
 }
 
@@ -68,7 +59,7 @@ func NewMaskinportenClientReconciler(
 // For more details, check Reconcile and its Result here:
 // - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.17.0/pkg/reconcile
 func (r *MaskinportenClientReconciler) Reconcile(ctx context.Context, kreq ctrl.Request) (ctrl.Result, error) {
-	ctx, span := r.tracer.Start(
+	ctx, span := r.runtime.Tracer().Start(
 		ctx,
 		"Reconcile",
 		trace.WithAttributes(attribute.String("namespace", kreq.Namespace), attribute.String("name", kreq.Name)),
@@ -154,7 +145,7 @@ func (r *MaskinportenClientReconciler) updateStatus(
 	reason string,
 	actions reconciliationActionList,
 ) error {
-	ctx, span := r.tracer.Start(ctx, "Reconcile.updateStatus")
+	ctx, span := r.runtime.Tracer().Start(ctx, "Reconcile.updateStatus")
 	defer span.End()
 
 	log := log.FromContext(ctx)
@@ -216,7 +207,7 @@ func (r *MaskinportenClientReconciler) getInstance(
 	ctx context.Context,
 	req *maskinportenClientRequest,
 ) (*resourcesv1alpha1.MaskinportenClient, error) {
-	ctx, span := r.tracer.Start(ctx, "Reconcile.getInstance")
+	ctx, span := r.runtime.Tracer().Start(ctx, "Reconcile.getInstance")
 	defer span.End()
 
 	instance := &resourcesv1alpha1.MaskinportenClient{}
@@ -245,7 +236,7 @@ func (r *MaskinportenClientReconciler) computeDesiredState(
 	req *maskinportenClientRequest,
 	instance *resourcesv1alpha1.MaskinportenClient,
 ) (maskinportenResourceList, error) {
-	_, span := r.tracer.Start(ctx, "Reconcile.computeDesiredState")
+	_, span := r.runtime.Tracer().Start(ctx, "Reconcile.computeDesiredState")
 	defer span.End()
 
 	resources := make(maskinportenResourceList, 0)
@@ -290,7 +281,7 @@ func (r *MaskinportenClientReconciler) fetchCurrentState(
 	ctx context.Context,
 	req *maskinportenClientRequest,
 ) (maskinportenResourceList, error) {
-	ctx, span := r.tracer.Start(ctx, "Reconcile.fetchCurrentState")
+	ctx, span := r.runtime.Tracer().Start(ctx, "Reconcile.fetchCurrentState")
 	defer span.End()
 
 	resources := make(maskinportenResourceList, 0)
@@ -338,7 +329,7 @@ func (r *MaskinportenClientReconciler) reconcile(
 	currentState maskinportenResourceList,
 	desiredState maskinportenResourceList,
 ) (reconciliationActionList, error) {
-	ctx, span := r.tracer.Start(ctx, "Reconcile.reconcile")
+	ctx, span := r.runtime.Tracer().Start(ctx, "Reconcile.reconcile")
 	defer span.End()
 
 	actions := make(reconciliationActionList, 0)

internal/controller/maskinportenclient_controller_test.go

@@ -12,6 +12,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	resourcesv1alpha1 "github.com/altinn/altinn-k8s-operator/api/v1alpha1"
+	"github.com/altinn/altinn-k8s-operator/internal"
 )
 
 var _ = Describe("MaskinportenClient Controller", func() {
@@ -52,13 +53,15 @@ var _ = Describe("MaskinportenClient Controller", func() {
 		})
 		It("should successfully reconcile the resource", func() {
 			By("Reconciling the created resource")
+			rt, err := internal.NewRuntime(context.Background())
+			Expect(err).NotTo(HaveOccurred())
 			controllerReconciler := NewMaskinportenClientReconciler(
-				context.Background(),
+				rt,
 				k8sClient,
 				k8sClient.Scheme(),
 			)
 
-			_, err := controllerReconciler.Reconcile(ctx, reconcile.Request{
+			_, err = controllerReconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: typeNamespacedName,
 			})
 			Expect(err).To(HaveOccurred())

internal/controller/request.go

@@ -43,7 +43,7 @@ func (r *MaskinportenClientReconciler) mapRequest(
 	ctx context.Context,
 	req ctrl.Request,
 ) (*maskinportenClientRequest, error) {
-	_, span := r.tracer.Start(ctx, "Reconcile.mapRequest")
+	_, span := r.runtime.Tracer().Start(ctx, "Reconcile.mapRequest")
 	defer span.End()
 
 	nameSplit := strings.Split(req.Name, "-")
@@ -59,6 +59,6 @@ func (r *MaskinportenClientReconciler) mapRequest(
 		Name:           req.Name,
 		Namespace:      req.Namespace,
 		AppId:          appId,
-		AppLabel:       fmt.Sprintf("%s-%s-deployment", operatorContext.Te, appId),
+		AppLabel:       fmt.Sprintf("%s-%s-deployment", operatorContext.ServiceOwner, appId),
 	}, nil
 }

internal/controller/resources.go

@@ -89,8 +89,8 @@ type reconciliationActionList []*reconciliationAction
 
 func (l *reconciliationActionList) Strings() []string {
 	res := make([]string, len(*l))
-	for _, a := range *l {
-		res = append(res, a.String())
+	for i, a := range *l {
+		res[i] = a.String()
 	}
 	return res
 }

internal/internal.go

@@ -7,18 +7,28 @@ import (
 	"github.com/altinn/altinn-k8s-operator/internal/maskinporten"
 	"github.com/altinn/altinn-k8s-operator/internal/operatorcontext"
 	rt "github.com/altinn/altinn-k8s-operator/internal/runtime"
+	"github.com/altinn/altinn-k8s-operator/internal/telemetry"
 	"github.com/jonboulle/clockwork"
+	"go.opentelemetry.io/otel"
+	"go.opentelemetry.io/otel/metric"
+	"go.opentelemetry.io/otel/trace"
 )
 
 type runtime struct {
 	config          config.Config
 	operatorContext operatorcontext.Context
 	clientManager   maskinporten.ClientManager
+	tracer          trace.Tracer
+	meter           metric.Meter
 }
 
 var _ rt.Runtime = (*runtime)(nil)
 
 func NewRuntime(ctx context.Context) (rt.Runtime, error) {
+	tracer := otel.Tracer(telemetry.ServiceName)
+	ctx, span := tracer.Start(ctx, "NewRuntime")
+	defer span.End()
+
 	operatorContext, err := operatorcontext.Discover(ctx)
 	if err != nil {
 		return nil, err
@@ -39,6 +49,8 @@ func NewRuntime(ctx context.Context) (rt.Runtime, error) {
 		config:          *cfg,
 		operatorContext: *operatorContext,
 		clientManager:   clientManager,
+		tracer:          tracer,
+		meter:           otel.Meter(telemetry.ServiceName),
 	}
 
 	return rt, nil
@@ -55,3 +67,11 @@ func (r *runtime) GetOperatorContext() *operatorcontext.Context {
 func (r *runtime) GetMaskinportenClientManager() maskinporten.ClientManager {
 	return r.clientManager
 }
+
+func (r *runtime) Tracer() trace.Tracer {
+	return r.tracer
+}
+
+func (r *runtime) Meter() metric.Meter {
+	return r.meter
+}

internal/maskinporten/http_api_client.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/altinn/altinn-k8s-operator/internal/caching"
 	"github.com/altinn/altinn-k8s-operator/internal/config"
+	"github.com/altinn/altinn-k8s-operator/internal/telemetry"
 	"github.com/cenkalti/backoff/v4"
 	"github.com/go-jose/go-jose/v4"
 	"github.com/go-jose/go-jose/v4/jwt"
@@ -53,7 +54,7 @@ func newApiClient(config *config.MaskinportenApiConfig, clock clockwork.Clock) (
 		config: config,
 		client: http.Client{Transport: otelhttp.NewTransport(http.DefaultTransport)},
 		jwk:    jwk,
-		tracer: otel.Tracer("maskinporten.ApiClient"),
+		tracer: otel.Tracer(telemetry.ServiceName),
 	}
 
 	client.wellKnown = caching.NewCachedAtom(5*time.Minute, clock, client.wellKnownFetcher)