You've already forked opentelemetry-go
mirror of
https://github.com/open-telemetry/opentelemetry-go.git
synced 2026-06-03 18:35:08 +02:00
fix: add error handling for insecure HTTP endpoints with TLS client configuration (#7914)
## Summary This PR moves the `insecure + TLS config` validation into the core OTLP HTTP exporters in `opentelemetry-go` (trace, metric, and log), instead of relying on validation in external/config-wrapper code. This aligns with feedback from `open-telemetry/opentelemetry-go-contrib` PR https://github.com/open-telemetry/opentelemetry-go-contrib/pull/8560: these packages are maintained/released together, so the check should be enforced in this repo as well. ## What changed - Added exporter-level validation error: - `"insecure HTTP endpoint cannot use TLS client configuration"` - Enforced in: - `otlpmetrichttp` client construction - `otlploghttp` client construction - `otlptracehttp` client startup (`Start`, since `NewClient` does not return an error) - Added tests in all three exporters to cover: - Error when `WithInsecure()` and `WithTLSClientConfig(...)` are both set - No error when a custom `WithHTTPClient(...)` is provided (preserves existing precedence semantics) ## Behavior - Invalid config (`insecure` + TLS config) now fails fast directly in exporters. - Existing `WithHTTPClient` override behavior remains unchanged. ## Testing Ran: - `go test ./...` in `exporters/otlp/otlpmetric/otlpmetrichttp` - `go test ./...` in `exporters/otlp/otlplog/otlploghttp` - `go test ./...` in `exporters/otlp/otlptrace/otlptracehttp` ## Related - `open-telemetry/opentelemetry-go-contrib` PR (https://github.com/open-telemetry/opentelemetry-go-contrib/pull/8560) --------- Co-authored-by: Damien Mathieu <42@dmathieu.com>
This commit is contained in:
@@ -20,6 +20,9 @@ The next release will require at least [Go 1.25].
|
||||
- Update `Baggage` in `go.opentelemetry.io/otel/propagation` and `Parse` and `New` in `go.opentelemetry.io/otel/baggage` to comply with W3C Baggage specification limits.
|
||||
`New` and `Parse` now return partial baggage along with an error when limits are exceeded.
|
||||
Errors from baggage extraction are reported to the global error handler. (#7880)
|
||||
- Return an error when the endpoint is configured as insecure and with TLS configuration in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#7914)
|
||||
- Return an error when the endpoint is configured as insecure and with TLS configuration in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#7914)
|
||||
- Return an error when the endpoint is configured as insecure and with TLS configuration in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#7914)
|
||||
|
||||
<!-- Released section -->
|
||||
<!-- Don't change this section unless doing release -->
|
||||
|
||||
@@ -45,6 +45,8 @@ func newNoopClient() *client {
|
||||
|
||||
var exporterN atomic.Int64
|
||||
|
||||
var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration")
|
||||
|
||||
// nextExporterID returns the next unique ID for an exporter.
|
||||
func nextExporterID() int64 {
|
||||
const inc = 1
|
||||
@@ -53,6 +55,10 @@ func nextExporterID() int64 {
|
||||
|
||||
// newHTTPClient creates a new HTTP log client.
|
||||
func newHTTPClient(cfg config) (*client, error) {
|
||||
if cfg.insecure.Value && cfg.tlsCfg.Value != nil {
|
||||
return nil, errInsecureEndpointWithTLS
|
||||
}
|
||||
|
||||
hc := cfg.httpClient
|
||||
if hc == nil {
|
||||
hc = &http.Client{
|
||||
|
||||
@@ -747,6 +747,16 @@ func TestConfig(t *testing.T) {
|
||||
assert.Len(t, coll.Collect().Dump(), 1)
|
||||
})
|
||||
|
||||
t.Run("WithInsecureAndTLSClientConfig", func(t *testing.T) {
|
||||
exp, err := New(t.Context(),
|
||||
WithEndpoint("localhost:4318"),
|
||||
WithInsecure(),
|
||||
WithTLSClientConfig(&tls.Config{}),
|
||||
)
|
||||
require.ErrorIs(t, err, errInsecureEndpointWithTLS)
|
||||
assert.Nil(t, exp)
|
||||
})
|
||||
|
||||
t.Run("WithCustomUserAgent", func(t *testing.T) {
|
||||
key := http.CanonicalHeaderKey("user-agent")
|
||||
headers := map[string]string{key: "custom-user-agent"}
|
||||
|
||||
@@ -52,8 +52,14 @@ var ourTransport = &http.Transport{
|
||||
ExpectContinueTimeout: 1 * time.Second,
|
||||
}
|
||||
|
||||
var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration")
|
||||
|
||||
// newClient creates a new HTTP metric client.
|
||||
func newClient(cfg oconf.Config) (*client, error) {
|
||||
if cfg.Metrics.Insecure && cfg.Metrics.TLSCfg != nil {
|
||||
return nil, errInsecureEndpointWithTLS
|
||||
}
|
||||
|
||||
httpClient := cfg.Metrics.HTTPClient
|
||||
if httpClient == nil {
|
||||
httpClient = &http.Client{
|
||||
|
||||
@@ -238,6 +238,16 @@ func TestConfig(t *testing.T) {
|
||||
assert.Len(t, coll.Collect().Dump(), 1)
|
||||
})
|
||||
|
||||
t.Run("WithInsecureAndTLSClientConfig", func(t *testing.T) {
|
||||
exp, err := New(t.Context(),
|
||||
WithEndpoint("localhost:4318"),
|
||||
WithInsecure(),
|
||||
WithTLSClientConfig(&tls.Config{}),
|
||||
)
|
||||
require.ErrorIs(t, err, errInsecureEndpointWithTLS)
|
||||
assert.Nil(t, exp)
|
||||
})
|
||||
|
||||
t.Run("WithCustomUserAgent", func(t *testing.T) {
|
||||
key := http.CanonicalHeaderKey("user-agent")
|
||||
headers := map[string]string{key: "custom-user-agent"}
|
||||
|
||||
@@ -56,6 +56,8 @@ var ourTransport = &http.Transport{
|
||||
ExpectContinueTimeout: 1 * time.Second,
|
||||
}
|
||||
|
||||
var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration")
|
||||
|
||||
type client struct {
|
||||
name string
|
||||
cfg otlpconfig.SignalConfig
|
||||
@@ -110,6 +112,10 @@ func NewClient(opts ...Option) otlptrace.Client {
|
||||
|
||||
// Start does nothing in a HTTP client.
|
||||
func (c *client) Start(ctx context.Context) error {
|
||||
if c.cfg.Insecure && c.cfg.TLSCfg != nil {
|
||||
return errInsecureEndpointWithTLS
|
||||
}
|
||||
|
||||
// Initialize the instrumentation if not already done.
|
||||
//
|
||||
// Initialize here instead of NewClient to allow any errors to be passed
|
||||
|
||||
@@ -5,6 +5,7 @@ package otlptracehttp_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/tls"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
@@ -253,6 +254,17 @@ func TestTimeout(t *testing.T) {
|
||||
assert.ErrorContains(t, err, "Client.Timeout exceeded while awaiting headers")
|
||||
}
|
||||
|
||||
func TestInsecureWithTLSClientConfig(t *testing.T) {
|
||||
client := otlptracehttp.NewClient(
|
||||
otlptracehttp.WithEndpoint("localhost:4318"),
|
||||
otlptracehttp.WithInsecure(),
|
||||
otlptracehttp.WithTLSClientConfig(&tls.Config{}),
|
||||
)
|
||||
exp, err := otlptrace.New(t.Context(), client)
|
||||
require.ErrorContains(t, err, "insecure HTTP endpoint cannot use TLS client configuration")
|
||||
assert.Nil(t, exp)
|
||||
}
|
||||
|
||||
func TestNoRetry(t *testing.T) {
|
||||
mc := runMockCollector(t, mockCollectorConfig{
|
||||
InjectHTTPStatus: []int{http.StatusBadRequest},
|
||||
|
||||
Reference in New Issue
Block a user