Skip to content

Add endpointURL for bare IP:port values #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bramwelt merged 2 commits into from
Feb 16, 2026
+124 −6
Merged

Add endpointURL for bare IP:port values #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39ea8b6
Add endpointURL for bare IP:port values
bramwelt Feb 12, 2026
d67a00d
Remove os.Setenv from endpoint normalization
bramwelt Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions pkg/utils/otel.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,13 @@ func SetupOTelSDKWithConfig(ctx context.Context, cfg OTelConfig) (shutdown func(
err = errors.Join(inErr, shutdown(ctx))
}

// Normalize endpoint to include a URL scheme so WithEndpointURL can
// parse it. Bare IP:port values like "127.0.0.1:4317" cause url.Parse
// to fail with "first path segment in URL cannot contain colon".
if cfg.Endpoint != "" {
cfg.Endpoint = endpointURL(cfg.Endpoint, cfg.Insecure)
}
Comment on lines +191 to +196
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description says SetupOTelSDKWithConfig sets OTEL_EXPORTER_OTLP_ENDPOINT after normalizing, but this function only rewrites cfg.Endpoint and never updates the environment variable. Either update the PR description (and the nearby comments) or set the env var here if that behavior is required for SDK/env-based configuration.

Copilot uses AI. Check for mistakes.

// Create resource with service information.
res, err := newResource(cfg)
if err != nil {
Expand Down Expand Up @@ -276,6 +283,21 @@ func newPropagator(cfg OTelConfig) propagation.TextMapPropagator {
return propagation.NewCompositeTextMapPropagator(propagators...)
}

// endpointURL ensures the endpoint has a URL scheme. The OTel SDK internally
// reads OTEL_EXPORTER_OTLP_ENDPOINT and parses it with url.Parse, which fails
// for bare IP:port values like "127.0.0.1:4317" with "first path segment in
// URL cannot contain colon". Prepending a scheme based on the insecure flag
// produces a valid URL the SDK can parse.
Comment on lines +286 to +290
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The endpointURL doc comment claims the SDK reads OTEL_EXPORTER_OTLP_ENDPOINT and parses it, but in this codepath endpointURL is only used to prepare cfg.Endpoint for WithEndpointURL options. Consider rewording the comment to describe the actual call chain (or implement env var normalization if that’s the intent).

Suggested change
// endpointURL ensures the endpoint has a URL scheme. The OTel SDK internally
// reads OTEL_EXPORTER_OTLP_ENDPOINT and parses it with url.Parse, which fails
// for bare IP:port values like "127.0.0.1:4317" with "first path segment in
// URL cannot contain colon". Prepending a scheme based on the insecure flag
// produces a valid URL the SDK can parse.
// endpointURL ensures the endpoint has a URL scheme suitable for use with
// OTLP exporter WithEndpointURL options. These helpers expect an endpoint
// value that url.Parse can handle, so bare IP:port values like "127.0.0.1:4317"
// would otherwise fail with "first path segment in URL cannot contain colon".
// Prepending "http://" or "https://" based on the insecure flag yields a valid
// URL while preserving the original host and port.

Copilot uses AI. Check for mistakes.
func endpointURL(raw string, insecure bool) string {
if strings.Contains(raw, "://") {
return raw
}
if insecure {
return "http://" + raw
}
return "https://" + raw
Comment on lines +291 to +298
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endpointURL should probably strings.TrimSpace(raw) before checking/adding a scheme; otherwise values like "127.0.0.1:4317\n" or " localhost:4317" will be turned into an invalid URL and WithEndpointURL will fail to parse.

Copilot uses AI. Check for mistakes.
}

// newTraceProvider creates a TracerProvider with an OTLP exporter configured based on the protocol setting.
func newTraceProvider(ctx context.Context, cfg OTelConfig, res *resource.Resource) (*trace.TracerProvider, error) {
var exporter trace.SpanExporter
Expand All @@ -284,7 +306,7 @@ func newTraceProvider(ctx context.Context, cfg OTelConfig, res *resource.Resourc
if cfg.Protocol == OTelProtocolHTTP {
opts := []otlptracehttp.Option{}
if cfg.Endpoint != "" {
opts = append(opts, otlptracehttp.WithEndpoint(cfg.Endpoint))
opts = append(opts, otlptracehttp.WithEndpointURL(cfg.Endpoint))
}
if cfg.Insecure {
opts = append(opts, otlptracehttp.WithInsecure())
Expand All @@ -293,7 +315,7 @@ func newTraceProvider(ctx context.Context, cfg OTelConfig, res *resource.Resourc
} else {
opts := []otlptracegrpc.Option{}
if cfg.Endpoint != "" {
opts = append(opts, otlptracegrpc.WithEndpoint(cfg.Endpoint))
opts = append(opts, otlptracegrpc.WithEndpointURL(cfg.Endpoint))
}
if cfg.Insecure {
opts = append(opts, otlptracegrpc.WithInsecure())
Expand Down Expand Up @@ -323,7 +345,7 @@ func newMetricsProvider(ctx context.Context, cfg OTelConfig, res *resource.Resou
if cfg.Protocol == OTelProtocolHTTP {
opts := []otlpmetrichttp.Option{}
if cfg.Endpoint != "" {
opts = append(opts, otlpmetrichttp.WithEndpoint(cfg.Endpoint))
opts = append(opts, otlpmetrichttp.WithEndpointURL(cfg.Endpoint))
}
if cfg.Insecure {
opts = append(opts, otlpmetrichttp.WithInsecure())
Expand All @@ -332,7 +354,7 @@ func newMetricsProvider(ctx context.Context, cfg OTelConfig, res *resource.Resou
} else {
opts := []otlpmetricgrpc.Option{}
if cfg.Endpoint != "" {
opts = append(opts, otlpmetricgrpc.WithEndpoint(cfg.Endpoint))
opts = append(opts, otlpmetricgrpc.WithEndpointURL(cfg.Endpoint))
}
if cfg.Insecure {
opts = append(opts, otlpmetricgrpc.WithInsecure())
Expand Down Expand Up @@ -361,7 +383,7 @@ func newLoggerProvider(ctx context.Context, cfg OTelConfig, res *resource.Resour
if cfg.Protocol == OTelProtocolHTTP {
opts := []otlploghttp.Option{}
if cfg.Endpoint != "" {
opts = append(opts, otlploghttp.WithEndpoint(cfg.Endpoint))
opts = append(opts, otlploghttp.WithEndpointURL(cfg.Endpoint))
}
if cfg.Insecure {
opts = append(opts, otlploghttp.WithInsecure())
Expand All @@ -370,7 +392,7 @@ func newLoggerProvider(ctx context.Context, cfg OTelConfig, res *resource.Resour
} else {
opts := []otlploggrpc.Option{}
if cfg.Endpoint != "" {
opts = append(opts, otlploggrpc.WithEndpoint(cfg.Endpoint))
opts = append(opts, otlploggrpc.WithEndpointURL(cfg.Endpoint))
}
if cfg.Insecure {
opts = append(opts, otlploggrpc.WithInsecure())
Expand Down
96 changes: 96 additions & 0 deletions pkg/utils/otel_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package utils

import (
"context"

Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra blank line inside the standard-library import block isn’t gofmt/goimports style and may cause formatting/lint churn. Consider running gofmt (or remove the empty line) so imports remain consistently formatted.

Suggested change

Copilot uses AI. Check for mistakes.
"testing"
)

Expand Down Expand Up @@ -225,3 +226,98 @@ func TestOTelConstants(t *testing.T) {
t.Errorf("expected OTelExporterNone to be 'none', got %q", OTelExporterNone)
}
}

// TestEndpointURL verifies that endpointURL prepends the correct scheme
// when missing and preserves existing schemes.
func TestEndpointURL(t *testing.T) {
tests := []struct {
name string
raw string
insecure bool
want string
}{
{
name: "IP:port insecure",
raw: "127.0.0.1:4317",
insecure: true,
want: "http://127.0.0.1:4317",
},
{
name: "IP:port secure",
raw: "127.0.0.1:4317",
insecure: false,
want: "https://127.0.0.1:4317",
},
{
name: "localhost:port insecure",
raw: "localhost:4317",
insecure: true,
want: "http://localhost:4317",
},
{
name: "hostname without port",
raw: "collector",
insecure: true,
want: "http://collector",
},
{
name: "http URL preserved",
raw: "http://collector.example.com:4318",
insecure: false,
want: "http://collector.example.com:4318",
},
{
name: "https URL preserved",
raw: "https://collector.example.com:4318",
insecure: true,
want: "https://collector.example.com:4318",
},
{
name: "https URL with path preserved",
raw: "https://collector.example.com:4318/v1/traces",
insecure: false,
want: "https://collector.example.com:4318/v1/traces",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := endpointURL(tt.raw, tt.insecure)
if got != tt.want {
t.Errorf("endpointURL(%q, %t) = %q, want %q", tt.raw, tt.insecure, got, tt.want)
}
})
}
}

// TestSetupOTelSDKWithConfig_IPEndpoint verifies that SetupOTelSDKWithConfig
// normalizes a bare IP:port endpoint to include a scheme, preventing the
// "first path segment in URL cannot contain colon" error from the SDK.
func TestSetupOTelSDKWithConfig_IPEndpoint(t *testing.T) {
t.Setenv("OTEL_EXPORTER_OTLP_ENDPOINT", "127.0.0.1:4317")

cfg := OTelConfig{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
Protocol: OTelProtocolGRPC,
Endpoint: "127.0.0.1:4317",
Insecure: true,
Comment on lines +297 to +304
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test sets OTEL_EXPORTER_OTLP_ENDPOINT but then constructs cfg.Endpoint directly, so the env var doesn’t influence the behavior under test. Consider either (1) building cfg via OTelConfigFromEnv() to validate the end-to-end env->cfg->normalization flow, or (2) removing the t.Setenv line to avoid implying env var handling is being tested here.

Copilot uses AI. Check for mistakes.
TracesExporter: OTelExporterOTLP,
TracesSampleRatio: 1.0,
MetricsExporter: OTelExporterNone,
LogsExporter: OTelExporterNone,
Propagators: "tracecontext,baggage",
}

ctx := context.Background()
shutdown, err := SetupOTelSDKWithConfig(ctx, cfg)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if shutdown == nil {
t.Fatal("expected non-nil shutdown function")
}

_ = shutdown(ctx)
}
Loading