fix(#804): OTLP endpoint path, CLI retry, and flush warning

internal/telemetry/telemetry.go

@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"time"
 
 	"go.opentelemetry.io/otel/attribute"
 	"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp"
@@ -32,9 +33,20 @@ const TelemetryFile = "run-telemetry.jsonl"
 
 const scopeName = "github.com/fullsend-ai/fullsend/internal/telemetry"
 
+// cliRetry configures retry timing for a short-lived CLI process.
+// The SDK defaults (5 s initial backoff, 30 s max interval, 60 s elapsed)
+// are tuned for long-lived services; a CLI at exit has ~5 s to flush spans.
+// We use short intervals so at least one retry fits inside the budget.
+var cliRetry = otlptracehttp.WithRetry(otlptracehttp.RetryConfig{
+	Enabled:         true,
+	InitialInterval: 500 * time.Millisecond,
+	MaxInterval:     2 * time.Second,
+	MaxElapsedTime:  4 * time.Second,
+})
+
 // newOTLPExporter is a seam over exporter construction for tests.
 var newOTLPExporter = func(ctx context.Context, endpoint string) (sdktrace.SpanExporter, error) {
-	return otlptracehttp.New(ctx, otlptracehttp.WithEndpointURL(endpoint))
+	return otlptracehttp.New(ctx, otlptracehttp.WithEndpointURL(endpoint), cliRetry)
 }
 
 // Setup creates a TracerProvider with file and (optionally) OTLP exporters.
@@ -63,7 +75,7 @@ func Setup(dir string, serviceVersion string) (trace.Tracer, func(context.Contex
 		sdktrace.WithSpanProcessor(sdktrace.NewSimpleSpanProcessor(newFileExporter(f))),
 	}
 
-	if endpoint := endpointFromEnv(); endpoint != "" && !isExporterNone() {
+	if endpoint := resolveEndpoint(); endpoint != "" && !isExporterNone() {
 		if err := validateEndpoint(endpoint); err != nil {
 			fmt.Fprintf(os.Stderr, "fullsend: OTLP export skipped: %v\n", err)
 		} else if exp, err := newOTLPExporter(context.Background(), endpoint); err != nil {
@@ -79,18 +91,40 @@ func Setup(dir string, serviceVersion string) (trace.Tracer, func(context.Contex
 	tracer := tp.Tracer(scopeName, trace.WithInstrumentationVersion(serviceVersion))
 
 	cleanup := func(ctx context.Context) {
-		_ = tp.Shutdown(ctx)
-		_ = f.Close()
+		if err := tp.Shutdown(ctx); err != nil {
+			fmt.Fprintf(os.Stderr, "fullsend: OTLP flush incomplete: %v\n", err)
+		}
+		_ = f.Close() // file already flushed by SimpleSpanProcessor; close error is not actionable
 	}
 
 	return tracer, cleanup
 }
 
-func endpointFromEnv() string {
+// resolveEndpoint returns the OTLP endpoint URL following the OTLP spec:
+//   - OTEL_EXPORTER_OTLP_TRACES_ENDPOINT (signal-specific) is used verbatim.
+//   - OTEL_EXPORTER_OTLP_ENDPOINT (generic) gets /v1/traces appended.
+//
+// Named resolveEndpoint (not endpointFromEnv) because it performs URL
+// parsing and path construction beyond simple env-var reading.
+// protocolFromEnv retains the *FromEnv suffix since it only reads env vars.
+//
+// This mirrors the behaviour documented in distributed-tracing.md and the
+// OTLP specification.
+func resolveEndpoint() string {
 	if v := strings.TrimSpace(os.Getenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT")); v != "" {
-		return v
+		return v // signal-specific: used as-is per spec
+	}
+	base := strings.TrimSpace(os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT"))
+	if base == "" {
+		return ""
+	}
+	// Generic endpoint: append /v1/traces per the OTLP spec.
+	u, err := url.Parse(base)
+	if err != nil {
+		return base // let validateEndpoint reject it
 	}
-	return strings.TrimSpace(os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT"))
+	u.Path = strings.TrimRight(u.Path, "/") + "/v1/traces"
+	return u.String()
 }
 
 func isSDKDisabled() bool {

internal/telemetry/telemetry_test.go

@@ -79,9 +79,11 @@ func TestSetup_OTLPExporterSeam(t *testing.T) {
 	defer func() { newOTLPExporter = orig }()
 
 	var called bool
-	newOTLPExporter = func(_ context.Context, _ string) (sdktrace.SpanExporter, error) {
+	var gotEndpoint string
+	newOTLPExporter = func(_ context.Context, endpoint string) (sdktrace.SpanExporter, error) {
 		called = true
-		return orig(context.Background(), "http://localhost:4318")
+		gotEndpoint = endpoint
+		return orig(context.Background(), "http://localhost:4318/v1/traces")
 	}
 
 	t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://localhost:4318")
@@ -93,19 +95,23 @@ func TestSetup_OTLPExporterSeam(t *testing.T) {
 	cleanup(context.Background())
 
 	assert.True(t, called, "OTLP exporter must be created when endpoint is set")
+	assert.Equal(t, "http://localhost:4318/v1/traces", gotEndpoint,
+		"generic endpoint must have /v1/traces appended")
 }
 
 func TestSetup_TracesEndpointPreferred(t *testing.T) {
 	orig := newOTLPExporter
 	defer func() { newOTLPExporter = orig }()
 
 	var called bool
-	newOTLPExporter = func(_ context.Context, _ string) (sdktrace.SpanExporter, error) {
+	var gotEndpoint string
+	newOTLPExporter = func(_ context.Context, endpoint string) (sdktrace.SpanExporter, error) {
 		called = true
-		return orig(context.Background(), "http://localhost:4318")
+		gotEndpoint = endpoint
+		return orig(context.Background(), "http://localhost:4318/v1/traces")
 	}
 
-	t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://traces.local:4318")
+	t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "http://traces.local:4318/v1/traces")
 	t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "http://generic.local:4318")
 	t.Setenv("OTEL_SDK_DISABLED", "")
 	t.Setenv("OTEL_TRACES_EXPORTER", "")
@@ -115,6 +121,8 @@ func TestSetup_TracesEndpointPreferred(t *testing.T) {
 	cleanup(context.Background())
 
 	assert.True(t, called, "OTLP exporter created when traces-specific endpoint set")
+	assert.Equal(t, "http://traces.local:4318/v1/traces", gotEndpoint,
+		"signal-specific endpoint must be used verbatim (no /v1/traces appended)")
 }
 
 func TestSetup_InvalidEndpointSkipsOTLP(t *testing.T) {
@@ -247,6 +255,68 @@ func TestParentSampledProcessor_AllowsSampledTrace(t *testing.T) {
 	assert.ElementsMatch(t, []string{"root", "child"}, spy.ended)
 }
 
+func TestResolveEndpoint_GenericAppendPath(t *testing.T) {
+	tests := []struct {
+		name    string
+		generic string
+		want    string
+	}{
+		{"bare host:port", "http://collector:4318", "http://collector:4318/v1/traces"},
+		{"trailing slash", "http://collector:4318/", "http://collector:4318/v1/traces"},
+		{"path prefix", "http://collector:4318/otlp", "http://collector:4318/otlp/v1/traces"},
+		{"path prefix trailing slash", "http://collector:4318/otlp/", "http://collector:4318/otlp/v1/traces"},
+		{"https", "https://otel.example.com", "https://otel.example.com/v1/traces"},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", tc.generic)
+			t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "")
+			assert.Equal(t, tc.want, resolveEndpoint())
+		})
+	}
+}
+
+func TestResolveEndpoint_SignalSpecificVerbatim(t *testing.T) {
+	tests := []struct {
+		name    string
+		traces  string
+		generic string
+		want    string
+	}{
+		{
+			"signal-specific used as-is",
+			"http://traces.local:4318/v1/traces",
+			"http://generic.local:4318",
+			"http://traces.local:4318/v1/traces",
+		},
+		{
+			"signal-specific with custom path",
+			"http://traces.local:4318/custom/path",
+			"http://generic.local:4318",
+			"http://traces.local:4318/custom/path",
+		},
+		{
+			"signal-specific bare",
+			"http://traces.local:4318",
+			"http://generic.local:4318",
+			"http://traces.local:4318",
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", tc.traces)
+			t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", tc.generic)
+			assert.Equal(t, tc.want, resolveEndpoint())
+		})
+	}
+}
+
+func TestResolveEndpoint_Empty(t *testing.T) {
+	t.Setenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", "")
+	t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "")
+	assert.Equal(t, "", resolveEndpoint())
+}
+
 func TestSetup_OTLPExporterError(t *testing.T) {
 	orig := newOTLPExporter
 	defer func() { newOTLPExporter = orig }()