Fix traces URL endpoint (#953)

grafana · Jun 21, 2024 · cc5a468 · cc5a468
1 parent b672ca0
commit cc5a468
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 20 deletions.
diff --git a/pkg/internal/export/otel/common.go b/pkg/internal/export/otel/common.go
@@ -120,8 +120,12 @@ func (rp *ReporterPool[T]) For(service svc.ID) (T, error) {
 
 // Intermediate representation of option functions suitable for testing
 type otlpOptions struct {
-	Endpoint      string
-	Insecure      bool
+	Scheme   string
+	Endpoint string
+	Insecure bool
+	// BaseURLPath, only for traces export, excludes the /v1/traces suffix.
+	// E.g. for a URLPath == "/otlp/v1/traces", BaseURLPath will be = "/otlp"
+	BaseURLPath   string
 	URLPath       string
 	SkipTLSVerify bool
 	HTTPHeaders   map[string]string

diff --git a/pkg/internal/export/otel/traces.go b/pkg/internal/export/otel/traces.go
@@ -196,22 +196,18 @@ func getTracesExporter(ctx context.Context, cfg TracesConfig, ctxInfo *global.Co
 			slog.Error("can't instantiate OTEL HTTP traces exporter", err)
 			return nil, err
 		}
-		endpoint, _, err := parseTracesEndpoint(&cfg)
-		if err != nil {
-			slog.Error("can't parse traces endpoint", "error", err)
-			return nil, err
-		}
 		factory := otlphttpexporter.NewFactory()
 		config := factory.CreateDefaultConfig().(*otlphttpexporter.Config)
 		config.QueueConfig.Enabled = false
 		config.ClientConfig = confighttp.ClientConfig{
-			Endpoint: endpoint.String(),
+			Endpoint: opts.Scheme + "://" + opts.Endpoint + opts.BaseURLPath,
 			TLSSetting: configtls.ClientConfig{
 				Insecure:           opts.Insecure,
 				InsecureSkipVerify: cfg.InsecureSkipVerify,
 			},
 			Headers: convertHeaders(opts.HTTPHeaders),
 		}
+		slog.Debug("getTracesExporter: confighttp.ClientConfig created", "endpoint", config.ClientConfig.Endpoint)
 		set := getTraceSettings(ctxInfo, cfg, t)
 		return factory.CreateTracesExporter(ctx, set, config)
 	case ProtocolGRPC:
@@ -640,20 +636,18 @@ func getHTTPTracesEndpointOptions(cfg *TracesConfig) (otlpOptions, error) {
 	log.Debug("Configuring exporter", "protocol",
 		cfg.Protocol, "tracesProtocol", cfg.TracesProtocol, "endpoint", murl.Host)
 	setTracesProtocol(cfg)
+	opts.Scheme = murl.Scheme
 	opts.Endpoint = murl.Host
 	if murl.Scheme == "http" || murl.Scheme == "unix" {
 		log.Debug("Specifying insecure connection", "scheme", murl.Scheme)
 		opts.Insecure = true
 	}
 	// If the value is set from the OTEL_EXPORTER_OTLP_ENDPOINT common property, we need to add /v1/traces to the path
 	// otherwise, we leave the path that is explicitly set by the user
-	opts.URLPath = murl.Path
+	opts.URLPath = strings.TrimSuffix(murl.Path, "/")
+	opts.BaseURLPath = strings.TrimSuffix(opts.URLPath, "/v1/traces")
 	if isCommon {
-		if strings.HasSuffix(opts.URLPath, "/") {
-			opts.URLPath += "v1/traces"
-		} else {
-			opts.URLPath += "/v1/traces"
-		}
+		opts.URLPath += "/v1/traces"
 		log.Debug("Specifying path", "path", opts.URLPath)
 	}
 

diff --git a/pkg/internal/export/otel/traces_test.go b/pkg/internal/export/otel/traces_test.go
@@ -38,23 +38,23 @@ func TestHTTPTracesEndpoint(t *testing.T) {
 	}
 
 	t.Run("testing with two endpoints", func(t *testing.T) {
-		testHTTPTracesOptions(t, otlpOptions{Endpoint: "localhost:3232", URLPath: "/v1/traces", HTTPHeaders: map[string]string{}}, &tcfg)
+		testHTTPTracesOptions(t, otlpOptions{Scheme: "https", Endpoint: "localhost:3232", URLPath: "/v1/traces", HTTPHeaders: map[string]string{}}, &tcfg)
 	})
 
 	tcfg = TracesConfig{
 		CommonEndpoint: "https://localhost:3131/otlp",
 	}
 
 	t.Run("testing with only common endpoint", func(t *testing.T) {
-		testHTTPTracesOptions(t, otlpOptions{Endpoint: "localhost:3131", URLPath: "/otlp/v1/traces", HTTPHeaders: map[string]string{}}, &tcfg)
+		testHTTPTracesOptions(t, otlpOptions{Scheme: "https", Endpoint: "localhost:3131", BaseURLPath: "/otlp", URLPath: "/otlp/v1/traces", HTTPHeaders: map[string]string{}}, &tcfg)
 	})
 
 	tcfg = TracesConfig{
 		CommonEndpoint: "https://localhost:3131",
 		TracesEndpoint: "http://localhost:3232",
 	}
 	t.Run("testing with insecure endpoint", func(t *testing.T) {
-		testHTTPTracesOptions(t, otlpOptions{Endpoint: "localhost:3232", Insecure: true, HTTPHeaders: map[string]string{}}, &tcfg)
+		testHTTPTracesOptions(t, otlpOptions{Scheme: "http", Endpoint: "localhost:3232", Insecure: true, HTTPHeaders: map[string]string{}}, &tcfg)
 	})
 
 	tcfg = TracesConfig{
@@ -63,7 +63,7 @@ func TestHTTPTracesEndpoint(t *testing.T) {
 	}
 
 	t.Run("testing with skip TLS verification", func(t *testing.T) {
-		testHTTPTracesOptions(t, otlpOptions{Endpoint: "localhost:3232", URLPath: "/v1/traces", SkipTLSVerify: true, HTTPHeaders: map[string]string{}}, &tcfg)
+		testHTTPTracesOptions(t, otlpOptions{Scheme: "https", Endpoint: "localhost:3232", URLPath: "/v1/traces", SkipTLSVerify: true, HTTPHeaders: map[string]string{}}, &tcfg)
 	})
 }
 
@@ -77,8 +77,10 @@ func TestHTTPTracesWithGrafanaOptions(t *testing.T) {
 	}}
 	t.Run("testing basic Grafana Cloud options", func(t *testing.T) {
 		testHTTPTracesOptions(t, otlpOptions{
-			Endpoint: "otlp-gateway-eu-west-23.grafana.net",
-			URLPath:  "/otlp/v1/traces",
+			Scheme:      "https",
+			Endpoint:    "otlp-gateway-eu-west-23.grafana.net",
+			BaseURLPath: "/otlp",
+			URLPath:     "/otlp/v1/traces",
 			HTTPHeaders: map[string]string{
 				// Basic + output of: echo -n 12345:affafafaafkd | gbase64 -w 0
 				"Authorization": "Basic MTIzNDU6YWZmYWZhZmFhZmtk",
@@ -88,6 +90,7 @@ func TestHTTPTracesWithGrafanaOptions(t *testing.T) {
 	mcfg.CommonEndpoint = "https://localhost:3939"
 	t.Run("Overriding endpoint URL", func(t *testing.T) {
 		testHTTPTracesOptions(t, otlpOptions{
+			Scheme:   "https",
 			Endpoint: "localhost:3939",
 			URLPath:  "/v1/traces",
 			HTTPHeaders: map[string]string{