From 242b777547b7076e462e0877dd07b917a01b207c Mon Sep 17 00:00:00 2001
From: Joshua Timmons <josh.timmons@hashicorp.com>
Date: Thu, 8 Feb 2024 11:00:47 -0500
Subject: [PATCH] Fix logging when we fail to export metrics to hcp (#20514)

---
 .changelog/20514.txt   |  3 +++
 agent/hcp/deps.go      | 22 +++++++++++++++++++---
 agent/hcp/deps_test.go |  2 +-
 3 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 .changelog/20514.txt

diff --git a/.changelog/20514.txt b/.changelog/20514.txt
new file mode 100644
index 0000000000..45eedfd82c
--- /dev/null
+++ b/.changelog/20514.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+hcp: fix error logs when failing to push metrics
+```
diff --git a/agent/hcp/deps.go b/agent/hcp/deps.go
index 5027c88ac7..05e65708b6 100644
--- a/agent/hcp/deps.go
+++ b/agent/hcp/deps.go
@@ -9,6 +9,7 @@ import (
 
 	"github.com/armon/go-metrics"
 	"github.com/hashicorp/go-hclog"
+	"go.opentelemetry.io/otel"
 
 	"github.com/hashicorp/consul/agent/hcp/client"
 	"github.com/hashicorp/consul/agent/hcp/config"
@@ -42,7 +43,7 @@ func NewDeps(cfg config.CloudConfig, logger hclog.Logger, dataDir string) (Deps,
 
 	metricsClient := client.NewMetricsClient(ctx, metricsProvider)
 
-	sink, err := sink(ctx, metricsClient, metricsProvider)
+	sink, err := newSink(ctx, metricsClient, metricsProvider)
 	if err != nil {
 		// Do not prevent server start if sink init fails, only log error.
 		logger.Error("failed to init sink", "error", err)
@@ -57,15 +58,22 @@ func NewDeps(cfg config.CloudConfig, logger hclog.Logger, dataDir string) (Deps,
 	}, nil
 }
 
-// sink initializes an OTELSink which forwards Consul metrics to HCP.
+// newSink initializes an OTELSink which forwards Consul metrics to HCP.
 // This step should not block server initialization, errors are returned, only to be logged.
-func sink(
+func newSink(
 	ctx context.Context,
 	metricsClient telemetry.MetricsClient,
 	cfgProvider *hcpProviderImpl,
 ) (metrics.ShutdownSink, error) {
 	logger := hclog.FromContext(ctx)
 
+	// Set the global OTEL error handler. Without this, on any failure to publish metrics in
+	// otelExporter.Export, the default OTEL handler logs to stderr without the formatting or group
+	// that hclog provides. Here we override that global error handler once so logs are
+	// in the standard format and include "hcp" in the group name like:
+	// 2024-02-06T22:35:19.072Z [ERROR] agent.hcp: failed to export metrics: failed to export metrics: code 404: 404 page not found
+	otel.SetErrorHandler(&otelErrorHandler{logger: logger})
+
 	reader := telemetry.NewOTELReader(metricsClient, cfgProvider)
 	sinkOpts := &telemetry.OTELSinkOpts{
 		Reader:         reader,
@@ -81,3 +89,11 @@ func sink(
 
 	return sink, nil
 }
+
+type otelErrorHandler struct {
+	logger hclog.Logger
+}
+
+func (o *otelErrorHandler) Handle(err error) {
+	o.logger.Error(err.Error())
+}
diff --git a/agent/hcp/deps_test.go b/agent/hcp/deps_test.go
index 7375dad68c..84a34dd697 100644
--- a/agent/hcp/deps_test.go
+++ b/agent/hcp/deps_test.go
@@ -21,7 +21,7 @@ func TestSink(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
-	s, err := sink(ctx, mockMetricsClient{}, &hcpProviderImpl{})
+	s, err := newSink(ctx, mockMetricsClient{}, &hcpProviderImpl{})
 
 	require.NotNil(t, s)
 	require.NoError(t, err)