fix: Inject all metrics and config later

cmd/refinery/main.go

@@ -188,6 +188,7 @@ func main() {
 	upstreamTransmission := transmit.NewDefaultTransmission(upstreamClient, upstreamMetricsRecorder, "upstream")
 	peerTransmission := transmit.NewDefaultTransmission(peerClient, peerMetricsRecorder, "peer")
 
+	// we need to include all the metrics types so we can inject them in case they're needed
 	var g inject.Graph
 	if opts.Debug {
 		g.Logger = graphLogger{}
@@ -202,6 +203,9 @@ func main() {
 		{Value: peerTransmission, Name: "peerTransmission"},
 		{Value: shrdr},
 		{Value: collector},
+		{Value: &metrics.LegacyMetrics{}, Name: "legacyMetrics"},
+		{Value: &metrics.PromMetrics{}, Name: "promMetrics"},
+		{Value: &metrics.OTelMetrics{}, Name: "otelMetrics"},
 		{Value: metricsSingleton, Name: "metrics"},
 		{Value: genericMetricsRecorder, Name: "genericMetrics"},
 		{Value: upstreamMetricsRecorder, Name: "upstreamMetrics"},
@@ -211,10 +215,6 @@ func main() {
 		{Value: stressRelief, Name: "stressRelief"},
 		{Value: &a},
 	}
-	// we need to add the multimetrics children to the graph as well
-	for _, obj := range metricsSingleton.Children() {
-		objects = append(objects, &inject.Object{Value: obj})
-	}
 	err = g.Provide(objects...)
 	if err != nil {
 		fmt.Printf("failed to provide injection graph. error: %+v\n", err)
@@ -234,19 +234,6 @@ func main() {
 		os.Exit(1)
 	}
 
-	// We need to remove the upstream and peer transmissions from the list of
-	// objects to startstop. For reasons I haven't been able to figure out,
-	// startstop sometimes decides to start them before the rest of the objects
-	// are ready, which causes a panic. We have to do it this way because
-	// startstop deliberately randomizes the graph order.
-	var nonxmitObjects []*inject.Object
-	for _, obj := range g.Objects() {
-		if obj.Name == "upstreamTransmission" || obj.Name == "peerTransmission" {
-			continue
-		}
-		nonxmitObjects = append(nonxmitObjects, obj)
-	}
-
 	// the logger provided to startstop must be valid before any service is
 	// started, meaning it can't rely on injected configs. make a custom logger
 	// just for this step
@@ -257,13 +244,10 @@ func main() {
 	// we can stop all the objects in one call, but we need to start the
 	// transmissions manually.
 	defer startstop.Stop(g.Objects(), ststLogger)
-	if err := startstop.Start(nonxmitObjects, ststLogger); err != nil {
+	if err := startstop.Start(g.Objects(), ststLogger); err != nil {
 		fmt.Printf("failed to start injected dependencies. error: %+v\n", err)
 		os.Exit(1)
 	}
-	// now start these manually
-	upstreamTransmission.Start()
-	peerTransmission.Start()
 
 	// these have to be done after the injection (of metrics)
 	// these are the metrics that libhoney will emit; we preregister them so that they always appear

metrics/metrics.go

@@ -46,21 +46,7 @@ type Metrics interface {
 }
 
 func GetMetricsImplementation(c config.Config) *MultiMetrics {
-	multi := NewMultiMetrics()
-
-	if c.GetLegacyMetricsConfig().Enabled {
-		multi.AddChild(&LegacyMetrics{})
-	}
-
-	if c.GetPrometheusMetricsConfig().Enabled {
-		multi.AddChild(&PromMetrics{})
-	}
-
-	if c.GetOTelMetricsConfig().Enabled {
-		multi.AddChild(&OTelMetrics{})
-	}
-
-	return multi
+	return NewMultiMetrics()
 }
 
 func ConvertNumeric(val interface{}) float64 {

metrics/multi_metrics.go

@@ -1,6 +1,10 @@
 package metrics
 
-import "sync"
+import (
+	"sync"
+
+	"github.com/honeycombio/refinery/config"
+)
 
 // MultiMetrics is a metrics provider that sends metrics to at least one
 // underlying metrics provider (StoreMetrics). It can be configured to send
@@ -10,9 +14,13 @@ import "sync"
 // which can then be retrieved with Get(). This is for use with StressRelief. It
 // does not track histograms or counters, which are reset after each scrape.
 type MultiMetrics struct {
-	children []Metrics
-	values   map[string]float64
-	lock     sync.RWMutex
+	Config        config.Config `inject:""`
+	LegacyMetrics Metrics       `inject:"legacyMetrics"`
+	PromMetrics   Metrics       `inject:"promMetrics"`
+	OTelMetrics   Metrics       `inject:"otelMetrics"`
+	children      []Metrics
+	values        map[string]float64
+	lock          sync.RWMutex
 }
 
 func NewMultiMetrics() *MultiMetrics {
@@ -23,6 +31,22 @@ func NewMultiMetrics() *MultiMetrics {
 }
 
 func (m *MultiMetrics) Start() error {
+	// I really hate having to do it this way, but
+	// the injector can't handle configurable items, so
+	// we need to inject everything and then build the
+	// array of children conditionally.
+	if m.Config.GetLegacyMetricsConfig().Enabled {
+		m.AddChild(m.LegacyMetrics)
+	}
+
+	if m.Config.GetPrometheusMetricsConfig().Enabled {
+		m.AddChild(m.PromMetrics)
+	}
+
+	if m.Config.GetOTelMetricsConfig().Enabled {
+		m.AddChild(m.OTelMetrics)
+	}
+
 	return nil
 }
 

metrics/multi_metrics_test.go

@@ -2,11 +2,14 @@ package metrics
 
 import (
 	"fmt"
+	"net/http"
 	"os"
 	"testing"
 
 	"github.com/facebookgo/inject"
 	"github.com/facebookgo/startstop"
+	"github.com/honeycombio/refinery/config"
+	"github.com/honeycombio/refinery/logger"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -28,11 +31,15 @@ func getAndStartMultiMetrics(children ...Metrics) (*MultiMetrics, error) {
 		mm.AddChild(child)
 	}
 	objects := []*inject.Object{
+		{Value: "version", Name: "version"},
+		{Value: &http.Transport{}, Name: "upstreamTransport"},
+		{Value: &http.Transport{}, Name: "peerTransport"},
+		{Value: &LegacyMetrics{}, Name: "legacyMetrics"},
+		{Value: &PromMetrics{}, Name: "promMetrics"},
+		{Value: &OTelMetrics{}, Name: "otelMetrics"},
 		{Value: mm, Name: "metrics"},
-	}
-	// we need to add the multimetrics children to the graph as well
-	for i, obj := range mm.Children() {
-		objects = append(objects, &inject.Object{Value: obj, Name: fmt.Sprintf("metricsChild_%d", i)})
+		{Value: &config.MockConfig{}},
+		{Value: &logger.NullLogger{}},
 	}
 	g := inject.Graph{Logger: dummyLogger{}}
 	err := g.Provide(objects...)