Skip to content

Commit 66fc10d

Browse files
sandy2008dmathieu
andauthored
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 open-telemetry/opentelemetry-go-contrib#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 (open-telemetry/opentelemetry-go-contrib#8560) --------- Co-authored-by: Damien Mathieu <42@dmathieu.com>
1 parent 76e6eec commit 66fc10d

File tree

7 files changed

+53
-0
lines changed

7 files changed

+53
-0
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ The next release will require at least [Go 1.25].
2020
- 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.
2121
`New` and `Parse` now return partial baggage along with an error when limits are exceeded.
2222
Errors from baggage extraction are reported to the global error handler. (#7880)
23+
- Return an error when the endpoint is configured as insecure and with TLS configuration in `go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp`. (#7914)
24+
- Return an error when the endpoint is configured as insecure and with TLS configuration in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#7914)
25+
- Return an error when the endpoint is configured as insecure and with TLS configuration in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#7914)
2326

2427
<!-- Released section -->
2528
<!-- Don't change this section unless doing release -->

exporters/otlp/otlplog/otlploghttp/client.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ func newNoopClient() *client {
4545

4646
var exporterN atomic.Int64
4747

48+
var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration")
49+
4850
// nextExporterID returns the next unique ID for an exporter.
4951
func nextExporterID() int64 {
5052
const inc = 1
@@ -53,6 +55,10 @@ func nextExporterID() int64 {
5355

5456
// newHTTPClient creates a new HTTP log client.
5557
func newHTTPClient(cfg config) (*client, error) {
58+
if cfg.insecure.Value && cfg.tlsCfg.Value != nil {
59+
return nil, errInsecureEndpointWithTLS
60+
}
61+
5662
hc := cfg.httpClient
5763
if hc == nil {
5864
hc = &http.Client{

exporters/otlp/otlplog/otlploghttp/client_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,6 +747,16 @@ func TestConfig(t *testing.T) {
747747
assert.Len(t, coll.Collect().Dump(), 1)
748748
})
749749

750+
t.Run("WithInsecureAndTLSClientConfig", func(t *testing.T) {
751+
exp, err := New(t.Context(),
752+
WithEndpoint("localhost:4318"),
753+
WithInsecure(),
754+
WithTLSClientConfig(&tls.Config{}),
755+
)
756+
require.ErrorIs(t, err, errInsecureEndpointWithTLS)
757+
assert.Nil(t, exp)
758+
})
759+
750760
t.Run("WithCustomUserAgent", func(t *testing.T) {
751761
key := http.CanonicalHeaderKey("user-agent")
752762
headers := map[string]string{key: "custom-user-agent"}

exporters/otlp/otlpmetric/otlpmetrichttp/client.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,14 @@ var ourTransport = &http.Transport{
5252
ExpectContinueTimeout: 1 * time.Second,
5353
}
5454

55+
var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration")
56+
5557
// newClient creates a new HTTP metric client.
5658
func newClient(cfg oconf.Config) (*client, error) {
59+
if cfg.Metrics.Insecure && cfg.Metrics.TLSCfg != nil {
60+
return nil, errInsecureEndpointWithTLS
61+
}
62+
5763
httpClient := cfg.Metrics.HTTPClient
5864
if httpClient == nil {
5965
httpClient = &http.Client{

exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,16 @@ func TestConfig(t *testing.T) {
238238
assert.Len(t, coll.Collect().Dump(), 1)
239239
})
240240

241+
t.Run("WithInsecureAndTLSClientConfig", func(t *testing.T) {
242+
exp, err := New(t.Context(),
243+
WithEndpoint("localhost:4318"),
244+
WithInsecure(),
245+
WithTLSClientConfig(&tls.Config{}),
246+
)
247+
require.ErrorIs(t, err, errInsecureEndpointWithTLS)
248+
assert.Nil(t, exp)
249+
})
250+
241251
t.Run("WithCustomUserAgent", func(t *testing.T) {
242252
key := http.CanonicalHeaderKey("user-agent")
243253
headers := map[string]string{key: "custom-user-agent"}

exporters/otlp/otlptrace/otlptracehttp/client.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ var ourTransport = &http.Transport{
5656
ExpectContinueTimeout: 1 * time.Second,
5757
}
5858

59+
var errInsecureEndpointWithTLS = errors.New("insecure HTTP endpoint cannot use TLS client configuration")
60+
5961
type client struct {
6062
name string
6163
cfg otlpconfig.SignalConfig
@@ -110,6 +112,10 @@ func NewClient(opts ...Option) otlptrace.Client {
110112

111113
// Start does nothing in a HTTP client.
112114
func (c *client) Start(ctx context.Context) error {
115+
if c.cfg.Insecure && c.cfg.TLSCfg != nil {
116+
return errInsecureEndpointWithTLS
117+
}
118+
113119
// Initialize the instrumentation if not already done.
114120
//
115121
// Initialize here instead of NewClient to allow any errors to be passed

exporters/otlp/otlptrace/otlptracehttp/client_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package otlptracehttp_test
55

66
import (
77
"context"
8+
"crypto/tls"
89
"fmt"
910
"io"
1011
"net/http"
@@ -253,6 +254,17 @@ func TestTimeout(t *testing.T) {
253254
assert.ErrorContains(t, err, "Client.Timeout exceeded while awaiting headers")
254255
}
255256

257+
func TestInsecureWithTLSClientConfig(t *testing.T) {
258+
client := otlptracehttp.NewClient(
259+
otlptracehttp.WithEndpoint("localhost:4318"),
260+
otlptracehttp.WithInsecure(),
261+
otlptracehttp.WithTLSClientConfig(&tls.Config{}),
262+
)
263+
exp, err := otlptrace.New(t.Context(), client)
264+
require.ErrorContains(t, err, "insecure HTTP endpoint cannot use TLS client configuration")
265+
assert.Nil(t, exp)
266+
}
267+
256268
func TestNoRetry(t *testing.T) {
257269
mc := runMockCollector(t, mockCollectorConfig{
258270
InjectHTTPStatus: []int{http.StatusBadRequest},

0 commit comments

Comments
 (0)