Skip to content

Commit 8ceba13

Browse files
pellaredpellared
authored
otlpmetrichttp otlptracehttp: Do not parse non-protobuf reponses (#4719)
1 parent d968509 commit 8ceba13

File tree

8 files changed

+115
-5
lines changed

8 files changed

+115
-5
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1414
- Remove the deprecated `go.opentelemetry.io/otel/exporters/otlp/otlpmetric` module. (#4707)
1515
- Remove the deprecated `go.opentelemetry.io/otel/example/view` module. (#4708)
1616

17+
### Fixed
18+
19+
- Do not parse non-protobuf responses in `go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp`. (#4719)
20+
- Do not parse non-protobuf responses in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#4719)
21+
1722
## [1.20.0/0.43.0] 2023-11-10
1823

1924
This release brings a breaking change for custom trace API implementations. Some interfaces (`TracerProvider`, `Tracer`, `Span`) now embed the `go.opentelemetry.io/otel/trace/embedded` types. Implementors need to update their implementations based on what they want the default behavior to be. See the "API Implementations" section of the [trace API] package documentation for more information about how to accomplish this.

‎exporters/otlp/otlpmetric/otlpmetricgrpc/internal/otest/collector.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ func (e *HTTPResponseError) Unwrap() error { return e.Err }
195195

196196
// HTTPCollector is an OTLP HTTP server that collects all requests it receives.
197197
type HTTPCollector struct {
198+
plainTextResponse bool
199+
198200
headersMu sync.Mutex
199201
headers http.Header
200202
storage *Storage
@@ -217,7 +219,7 @@ type HTTPCollector struct {
217219
// If errCh is not nil, the collector will respond to HTTP requests with errors
218220
// sent on that channel. This means that if errCh is not nil Export calls will
219221
// block until an error is received.
220-
func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPCollector, error) {
222+
func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult, opts ...func(*HTTPCollector)) (*HTTPCollector, error) {
221223
u, err := url.Parse(endpoint)
222224
if err != nil {
223225
return nil, err
@@ -234,6 +236,9 @@ func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPColle
234236
storage: NewStorage(),
235237
resultCh: resultCh,
236238
}
239+
for _, opt := range opts {
240+
opt(c)
241+
}
237242

238243
c.listener, err = net.Listen("tcp", u.Host)
239244
if err != nil {
@@ -262,6 +267,14 @@ func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPColle
262267
return c, nil
263268
}
264269

270+
// WithHTTPCollectorRespondingPlainText makes the HTTPCollector return
271+
// a plaintext, instead of protobuf, response.
272+
func WithHTTPCollectorRespondingPlainText() func(*HTTPCollector) {
273+
return func(s *HTTPCollector) {
274+
s.plainTextResponse = true
275+
}
276+
}
277+
265278
// Shutdown shuts down the HTTP server closing all open connections and
266279
// listeners.
267280
func (c *HTTPCollector) Shutdown(ctx context.Context) error {
@@ -382,6 +395,13 @@ func (c *HTTPCollector) respond(w http.ResponseWriter, resp ExportResult) {
382395
return
383396
}
384397

398+
if c.plainTextResponse {
399+
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
400+
w.WriteHeader(http.StatusOK)
401+
_, _ = w.Write([]byte("OK"))
402+
return
403+
}
404+
385405
w.Header().Set("Content-Type", "application/x-protobuf")
386406
w.WriteHeader(http.StatusOK)
387407
if resp.Response == nil {

‎exporters/otlp/otlpmetric/otlpmetrichttp/client.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,8 +166,11 @@ func (c *client) UploadMetrics(ctx context.Context, protoMetrics *metricpb.Resou
166166
if _, err := io.Copy(&respData, resp.Body); err != nil {
167167
return err
168168
}
169+
if respData.Len() == 0 {
170+
return nil
171+
}
169172

170-
if respData.Len() != 0 {
173+
if resp.Header.Get("Content-Type") == "application/x-protobuf" {
171174
var respProto colmetricpb.ExportMetricsServiceResponse
172175
if err := proto.Unmarshal(respData.Bytes(), &respProto); err != nil {
173176
return err

‎exporters/otlp/otlpmetric/otlpmetrichttp/client_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import (
3131
"go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest"
3232
"go.opentelemetry.io/otel/sdk/metric"
3333
"go.opentelemetry.io/otel/sdk/metric/metricdata"
34+
mpb "go.opentelemetry.io/proto/otlp/metrics/v1"
3435
)
3536

3637
type clientShim struct {
@@ -65,6 +66,23 @@ func TestClient(t *testing.T) {
6566
t.Run("Integration", otest.RunClientTests(factory))
6667
}
6768

69+
func TestClientWithHTTPCollectorRespondingPlainText(t *testing.T) {
70+
ctx := context.Background()
71+
coll, err := otest.NewHTTPCollector("", nil, otest.WithHTTPCollectorRespondingPlainText())
72+
require.NoError(t, err)
73+
74+
addr := coll.Addr().String()
75+
opts := []Option{WithEndpoint(addr), WithInsecure()}
76+
cfg := oconf.NewHTTPConfig(asHTTPOptions(opts)...)
77+
client, err := newClient(cfg)
78+
require.NoError(t, err)
79+
80+
require.NoError(t, client.UploadMetrics(ctx, &mpb.ResourceMetrics{}))
81+
require.NoError(t, client.Shutdown(ctx))
82+
got := coll.Collect().Dump()
83+
require.Len(t, got, 1, "upload of one ResourceMetrics")
84+
}
85+
6886
func TestNewWithInvalidEndpoint(t *testing.T) {
6987
ctx := context.Background()
7088
exp, err := New(ctx, WithEndpoint("host:invalid-port"))

‎exporters/otlp/otlpmetric/otlpmetrichttp/internal/otest/collector.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ func (e *HTTPResponseError) Unwrap() error { return e.Err }
195195

196196
// HTTPCollector is an OTLP HTTP server that collects all requests it receives.
197197
type HTTPCollector struct {
198+
plainTextResponse bool
199+
198200
headersMu sync.Mutex
199201
headers http.Header
200202
storage *Storage
@@ -217,7 +219,7 @@ type HTTPCollector struct {
217219
// If errCh is not nil, the collector will respond to HTTP requests with errors
218220
// sent on that channel. This means that if errCh is not nil Export calls will
219221
// block until an error is received.
220-
func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPCollector, error) {
222+
func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult, opts ...func(*HTTPCollector)) (*HTTPCollector, error) {
221223
u, err := url.Parse(endpoint)
222224
if err != nil {
223225
return nil, err
@@ -234,6 +236,9 @@ func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPColle
234236
storage: NewStorage(),
235237
resultCh: resultCh,
236238
}
239+
for _, opt := range opts {
240+
opt(c)
241+
}
237242

238243
c.listener, err = net.Listen("tcp", u.Host)
239244
if err != nil {
@@ -262,6 +267,14 @@ func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPColle
262267
return c, nil
263268
}
264269

270+
// WithHTTPCollectorRespondingPlainText makes the HTTPCollector return
271+
// a plaintext, instead of protobuf, response.
272+
func WithHTTPCollectorRespondingPlainText() func(*HTTPCollector) {
273+
return func(s *HTTPCollector) {
274+
s.plainTextResponse = true
275+
}
276+
}
277+
265278
// Shutdown shuts down the HTTP server closing all open connections and
266279
// listeners.
267280
func (c *HTTPCollector) Shutdown(ctx context.Context) error {
@@ -382,6 +395,13 @@ func (c *HTTPCollector) respond(w http.ResponseWriter, resp ExportResult) {
382395
return
383396
}
384397

398+
if c.plainTextResponse {
399+
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
400+
w.WriteHeader(http.StatusOK)
401+
_, _ = w.Write([]byte("OK"))
402+
return
403+
}
404+
385405
w.Header().Set("Content-Type", "application/x-protobuf")
386406
w.WriteHeader(http.StatusOK)
387407
if resp.Response == nil {

‎exporters/otlp/otlptrace/otlptracehttp/client.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,11 @@ func (d *client) UploadTraces(ctx context.Context, protoSpans []*tracepb.Resourc
177177
if _, err := io.Copy(&respData, resp.Body); err != nil {
178178
return err
179179
}
180+
if respData.Len() == 0 {
181+
return nil
182+
}
180183

181-
if respData.Len() != 0 {
184+
if resp.Header.Get("Content-Type") == "application/x-protobuf" {
182185
var respProto coltracepb.ExportTraceServiceResponse
183186
if err := proto.Unmarshal(respData.Bytes(), &respProto); err != nil {
184187
return err

‎exporters/otlp/otlptrace/otlptracehttp/client_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,3 +430,24 @@ func TestOtherHTTPSuccess(t *testing.T) {
430430
})
431431
}
432432
}
433+
434+
func TestCollectorRespondingNonProtobufContent(t *testing.T) {
435+
mcCfg := mockCollectorConfig{
436+
InjectContentType: "application/octet-stream",
437+
}
438+
mc := runMockCollector(t, mcCfg)
439+
defer mc.MustStop(t)
440+
driver := otlptracehttp.NewClient(
441+
otlptracehttp.WithEndpoint(mc.Endpoint()),
442+
otlptracehttp.WithInsecure(),
443+
)
444+
ctx := context.Background()
445+
exporter, err := otlptrace.New(ctx, driver)
446+
require.NoError(t, err)
447+
defer func() {
448+
assert.NoError(t, exporter.Shutdown(context.Background()))
449+
}()
450+
err = exporter.ExportSpans(ctx, otlptracetest.SingleReadOnlySpan())
451+
assert.NoError(t, err)
452+
assert.Len(t, mc.GetSpans(), 1)
453+
}

‎internal/shared/otlp/otlpmetric/otest/collector.go.tmpl

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ func (e *HTTPResponseError) Unwrap() error { return e.Err }
195195

196196
// HTTPCollector is an OTLP HTTP server that collects all requests it receives.
197197
type HTTPCollector struct {
198+
plainTextResponse bool
199+
198200
headersMu sync.Mutex
199201
headers http.Header
200202
storage *Storage
@@ -217,7 +219,7 @@ type HTTPCollector struct {
217219
// If errCh is not nil, the collector will respond to HTTP requests with errors
218220
// sent on that channel. This means that if errCh is not nil Export calls will
219221
// block until an error is received.
220-
func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPCollector, error) {
222+
func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult, opts ...func(*HTTPCollector)) (*HTTPCollector, error) {
221223
u, err := url.Parse(endpoint)
222224
if err != nil {
223225
return nil, err
@@ -234,6 +236,9 @@ func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPColle
234236
storage: NewStorage(),
235237
resultCh: resultCh,
236238
}
239+
for _, opt := range opts {
240+
opt(c)
241+
}
237242

238243
c.listener, err = net.Listen("tcp", u.Host)
239244
if err != nil {
@@ -262,6 +267,14 @@ func NewHTTPCollector(endpoint string, resultCh <-chan ExportResult) (*HTTPColle
262267
return c, nil
263268
}
264269

270+
// WithHTTPCollectorRespondingPlainText makes the HTTPCollector return
271+
// a plaintext, instead of protobuf, response.
272+
func WithHTTPCollectorRespondingPlainText() func(*HTTPCollector) {
273+
return func(s *HTTPCollector) {
274+
s.plainTextResponse = true
275+
}
276+
}
277+
265278
// Shutdown shuts down the HTTP server closing all open connections and
266279
// listeners.
267280
func (c *HTTPCollector) Shutdown(ctx context.Context) error {
@@ -382,6 +395,13 @@ func (c *HTTPCollector) respond(w http.ResponseWriter, resp ExportResult) {
382395
return
383396
}
384397

398+
if c.plainTextResponse {
399+
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
400+
w.WriteHeader(http.StatusOK)
401+
_, _ = w.Write([]byte("OK"))
402+
return
403+
}
404+
385405
w.Header().Set("Content-Type", "application/x-protobuf")
386406
w.WriteHeader(http.StatusOK)
387407
if resp.Response == nil {

0 commit comments

Comments
 (0)