From 9a4ef3806f4885ccf9ce43f84695284dc49ea71e Mon Sep 17 00:00:00 2001
From: Jon Johnson <113393155+jonathanj-square@users.noreply.github.com>
Date: Wed, 24 Jul 2024 16:37:01 -0700
Subject: [PATCH] incorporating PR feedback

---
 backend/controller/controller.go              |  5 +++
 .../controller/observability/attributes.go    |  5 ---
 backend/controller/observability/fsm.go       | 36 +++++++++++++------
 .../controller/observability/observability.go | 10 ++++--
 .../observability/{metrics => }/attributes.go |  2 +-
 internal/observability/client.go              |  3 --
 6 files changed, 39 insertions(+), 22 deletions(-)
 delete mode 100644 backend/controller/observability/attributes.go
 rename internal/observability/{metrics => }/attributes.go (70%)

diff --git a/backend/controller/controller.go b/backend/controller/controller.go
index f05f7e72a7..662069abbe 100644
--- a/backend/controller/controller.go
+++ b/backend/controller/controller.go
@@ -7,6 +7,7 @@ import (
 	"encoding/binary"
 	"errors"
 	"fmt"
+	"github.com/TBD54566975/ftl/backend/controller/observability"
 	"hash"
 	"io"
 	"math/rand"
@@ -226,6 +227,10 @@ func New(ctx context.Context, pool *pgxpool.Pool, config Config, runnerScaling s
 	}
 	config.SetDefaults()
 
+	if err := observability.InitControllerObservability(); err != nil {
+		log.FromContext(ctx).Warnf("failed to initialize controller observability: %v", err)
+	}
+
 	// Override some defaults during development mode.
 	_, devel := runnerScaling.(*localscaling.LocalScaling)
 	if devel {
diff --git a/backend/controller/observability/attributes.go b/backend/controller/observability/attributes.go
deleted file mode 100644
index 5409d2d109..0000000000
--- a/backend/controller/observability/attributes.go
+++ /dev/null
@@ -1,5 +0,0 @@
-package observability
-
-const (
-	fsmRefAttribute string = "ftl.fsm.ref"
-)
diff --git a/backend/controller/observability/fsm.go b/backend/controller/observability/fsm.go
index afc68865f0..64da7e7935 100644
--- a/backend/controller/observability/fsm.go
+++ b/backend/controller/observability/fsm.go
@@ -4,13 +4,16 @@ import (
 	"context"
 	"fmt"
 	"github.com/TBD54566975/ftl/backend/schema"
-	"github.com/TBD54566975/ftl/internal/observability/metrics"
+	"github.com/TBD54566975/ftl/internal/observability"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/metric"
 )
 
-const fsmMeterName = "ftl.fsm"
+const (
+	fsmMeterName    = "ftl.fsm"
+	fsmRefAttribute = "ftl.fsm.ref"
+)
 
 var fsmMeter = otel.Meter("ftl.fsm")
 
@@ -18,21 +21,32 @@ var fsmCounters = struct {
 	instancesActive metric.Int64UpDownCounter
 }{}
 
-// TODO error logging and handling
-func InitFSMMetrics() {
-	fsmCounters.instancesActive, _ = fsmMeter.Int64UpDownCounter(
+func InitFSMMetrics() error {
+	var err error
+
+	fsmCounters.instancesActive, err = fsmMeter.Int64UpDownCounter(
 		fmt.Sprintf("%s.instances.active", fsmMeterName),
 		metric.WithDescription("counts the number of active FSM instances"))
+
+	if err != nil {
+		return fmt.Errorf("could not initialize fsm metrics: %w", err)
+	}
+
+	return nil
 }
 
 func FSMInstanceCreated(ctx context.Context, fsm schema.RefKey) {
-	fsmCounters.instancesActive.Add(ctx, 1, metric.WithAttributes(
-		attribute.String(metrics.ModuleNameAttribute, fsm.Module),
-		attribute.String(fsmRefAttribute, fsm.String())))
+	if fsmCounters.instancesActive != nil {
+		fsmCounters.instancesActive.Add(ctx, 1, metric.WithAttributes(
+			attribute.String(observability.ModuleNameAttribute, fsm.Module),
+			attribute.String(fsmRefAttribute, fsm.String())))
+	}
 }
 
 func FSMInstanceCompleted(ctx context.Context, fsm schema.RefKey) {
-	fsmCounters.instancesActive.Add(ctx, -1, metric.WithAttributes(
-		attribute.String(metrics.ModuleNameAttribute, fsm.Module),
-		attribute.String(fsmRefAttribute, fsm.String())))
+	if fsmCounters.instancesActive != nil {
+		fsmCounters.instancesActive.Add(ctx, -1, metric.WithAttributes(
+			attribute.String(observability.ModuleNameAttribute, fsm.Module),
+			attribute.String(fsmRefAttribute, fsm.String())))
+	}
 }
diff --git a/backend/controller/observability/observability.go b/backend/controller/observability/observability.go
index 34a13604f1..8d84df13f0 100644
--- a/backend/controller/observability/observability.go
+++ b/backend/controller/observability/observability.go
@@ -1,5 +1,11 @@
 package observability
 
-func Init() {
-	InitFSMMetrics()
+import "fmt"
+
+func InitControllerObservability() error {
+	if err := InitFSMMetrics(); err != nil {
+		return fmt.Errorf("could not initialize controller metrics: %w", err)
+	}
+
+	return nil
 }
diff --git a/internal/observability/metrics/attributes.go b/internal/observability/attributes.go
similarity index 70%
rename from internal/observability/metrics/attributes.go
rename to internal/observability/attributes.go
index 6df5019a1c..ff6bd6c558 100644
--- a/internal/observability/metrics/attributes.go
+++ b/internal/observability/attributes.go
@@ -1,4 +1,4 @@
-package metrics
+package observability
 
 const (
 	ModuleNameAttribute = "ftl.module.name"
diff --git a/internal/observability/client.go b/internal/observability/client.go
index 2e916adcbb..4d8f88d3f1 100644
--- a/internal/observability/client.go
+++ b/internal/observability/client.go
@@ -3,7 +3,6 @@ package observability
 import (
 	"context"
 	"fmt"
-	"github.com/TBD54566975/ftl/backend/controller/observability"
 	"os"
 	"strings"
 
@@ -65,8 +64,6 @@ func Init(ctx context.Context, serviceName, serviceVersion string, config Config
 	meterProvider := metric.NewMeterProvider(metric.WithReader(metric.NewPeriodicReader(otelMetricExporter)), metric.WithResource(res))
 	otel.SetMeterProvider(meterProvider)
 
-	observability.Init()
-
 	otelTraceExporter, err := otlptracegrpc.New(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to create OTEL trace exporter: %w", err)