fix: add timeout to jaeger report

Author: zimeg

internal/config/dotenv.go

@@ -19,18 +19,9 @@ import (
 
 	"github.com/joho/godotenv"
 	"github.com/slackapi/slack-cli/internal/pkg/version"
-	"github.com/slackapi/slack-cli/internal/shared/types"
 	"github.com/spf13/afero"
 )
 
-// IsTelemetryDisabled checks environment variables to determine if telemetry
-// should be disabled. This can be called before Config is initialized.
-func IsTelemetryDisabled(os types.Os) bool {
-	var disableTelemetry = strings.TrimSpace(os.Getenv(slackDisableTelemetryEnv))
-	var testVersion = strings.TrimSpace(os.Getenv(version.EnvTestVersion))
-	return (disableTelemetry != "" && disableTelemetry != "false" && disableTelemetry != "0") || testVersion != ""
-}
-
 // GetDotEnvFileVariables collects only the variables in the .env file
 func (c *Config) GetDotEnvFileVariables() (map[string]string, error) {
 	variables := map[string]string{}
@@ -69,7 +60,9 @@ func (c *Config) LoadEnvironmentVariables() error {
 	}
 
 	// Disable telemetry if either disable-telemetry or test-version environment variables
-	if IsTelemetryDisabled(c.os) {
+	var disableTelemetry = strings.TrimSpace(c.os.Getenv(slackDisableTelemetryEnv))
+	var testVersion = strings.TrimSpace(c.os.Getenv(version.EnvTestVersion))
+	if (disableTelemetry != "" && disableTelemetry != "false" && disableTelemetry != "0") || testVersion != "" {
 		c.DisableTelemetryFlag = true
 	}
 

internal/slackcontext/slackcontext_test.go

@@ -112,7 +112,7 @@ func Test_SlackContext_SetOpenTracingTraceID(t *testing.T) {
 }
 
 func Test_SlackContext_OpenTracingTracer(t *testing.T) {
-	_, _tracer := tracer.SetupTracer(true, false)
+	_, _tracer := tracer.SetupTracer(true)
 
 	tests := map[string]struct {
 		expectedTracer opentracing.Tracer
@@ -139,7 +139,7 @@ func Test_SlackContext_OpenTracingTracer(t *testing.T) {
 }
 
 func Test_SlackContext_SetOpenTracingTracer(t *testing.T) {
-	_, _tracer := tracer.SetupTracer(true, false)
+	_, _tracer := tracer.SetupTracer(true)
 
 	tests := map[string]struct {
 		expectedTracer opentracing.Tracer

internal/tracer/tracer.go

@@ -20,35 +20,38 @@ import (
 	"time"
 
 	"github.com/opentracing/opentracing-go"
+	jaeger "github.com/uber/jaeger-client-go"
 	jaegercfg "github.com/uber/jaeger-client-go/config"
+	"github.com/uber/jaeger-client-go/transport"
 )
 
 // SetupTracer sets up the tracer to send data to honeycomb
-func SetupTracer(isDev bool, disableTelemetry bool) (io.Closer, opentracing.Tracer) {
-	var collectorEndpoint = "https://slackb.com/traces/v1/jaeger"
+func SetupTracer(isDev bool) (io.Closer, opentracing.Tracer) {
+	collectorEndpoint := "https://slackb.com/traces/v1/jaeger"
 	if isDev {
 		collectorEndpoint = "https://dev.slackb.com/traces/v1/jaeger"
 	}
 	// Recommended configuration for production.
-	var jCfg = jaegercfg.Configuration{
+	jCfg := jaegercfg.Configuration{
 		ServiceName: "slack-cli", // Don't change this.  Required to distinguish logs & traces coming from the CLI
 		Disabled:    false,       // Keep tracer active so span contexts and trace IDs are still generated
-		Reporter: &jaegercfg.ReporterConfig{
-			LogSpans:            false,
-			CollectorEndpoint:   collectorEndpoint,
-			BufferFlushInterval: 100 * time.Millisecond,
-			// Having a larger value here results in longer execution of every single CLI command
-			// We may need to adjust the size here if we observe lost data in metrics.
-			QueueSize: 1,
-		},
 		Sampler: &jaegercfg.SamplerConfig{
 			Type:  "const",
 			Param: 1,
 		},
 	}
 
+	// Create HTTP transport with a short timeout to prevent hangs on unreachable hosts
+	sender := transport.NewHTTPTransport(collectorEndpoint, transport.HTTPTimeout(1*time.Second))
+	reporter := jaeger.NewRemoteReporter(sender,
+		jaeger.ReporterOptions.BufferFlushInterval(100*time.Millisecond),
+		// Having a larger value here results in longer execution of every single CLI command
+		// We may need to adjust the size here if we observe lost data in metrics.
+		jaeger.ReporterOptions.QueueSize(1),
+	)
+
 	// Initialize tracer with a logger and a metrics factory
-	var tracer, jaegerCloser, traceErr = jCfg.NewTracer()
+	tracer, jaegerCloser, traceErr := jCfg.NewTracer(jaegercfg.Reporter(reporter))
 	if traceErr != nil {
 		log.Fatalf("Could not initialize jaeger tracer: %s", traceErr.Error())
 	}

main.go

@@ -24,14 +24,12 @@ import (
 	"github.com/google/uuid"
 	"github.com/opentracing/opentracing-go"
 	"github.com/slackapi/slack-cli/cmd"
-	"github.com/slackapi/slack-cli/internal/config"
 	"github.com/slackapi/slack-cli/internal/goutils"
 	"github.com/slackapi/slack-cli/internal/iostreams"
 	"github.com/slackapi/slack-cli/internal/ioutils"
 	"github.com/slackapi/slack-cli/internal/pkg/version"
 	"github.com/slackapi/slack-cli/internal/shared"
 	"github.com/slackapi/slack-cli/internal/slackcontext"
-	"github.com/slackapi/slack-cli/internal/slackdeps"
 	"github.com/slackapi/slack-cli/internal/tracer"
 	"github.com/uber/jaeger-client-go"
 )
@@ -46,7 +44,7 @@ func main() {
 	//      - This would allow us to choose the correct API host based on flags
 	//      - Uncomment `isDevTarget` if we refactor to cmd/root.go and update to call `ResolveAPIHost`
 	// var isDevTarget = shared.NewClientFactory().AuthClient().UserDefaultAuthIsProd(ctx) // TODO - hack, remove shared.clients
-	var jaegerCloser, tracer = tracer.SetupTracer(false, config.IsTelemetryDisabled(slackdeps.NewOs()))
+	var jaegerCloser, tracer = tracer.SetupTracer(false)
 	defer func() {
 		done := make(chan struct{})
 		go func() {