diff --git a/CHANGELOG.md b/CHANGELOG.md index d1823333836..058deb3fa79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Move the OpenCensus example into `example` directory. (#1359) - `NewExporter` and `Start` functions in `go.opentelemetry.io/otel/exporters/otlp` now receive `context.Context` as a first parameter. (#1357) +- Zipkin exporter relies on the status code for success rather than body read but still read the response body. (#1328) ## [0.14.0] - 2020-11-19 diff --git a/exporters/trace/zipkin/zipkin.go b/exporters/trace/zipkin/zipkin.go index 51f57e37d58..935331451a9 100644 --- a/exporters/trace/zipkin/zipkin.go +++ b/exporters/trace/zipkin/zipkin.go @@ -20,6 +20,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "io/ioutil" "log" "net/http" @@ -112,14 +113,14 @@ func NewRawExporter(collectorURL, serviceName string, opts ...Option) (*Exporter // NewExportPipeline sets up a complete export pipeline // with the recommended setup for trace provider func NewExportPipeline(collectorURL, serviceName string, opts ...Option) (*sdktrace.TracerProvider, error) { - exp, err := NewRawExporter(collectorURL, serviceName, opts...) + exporter, err := NewRawExporter(collectorURL, serviceName, opts...) if err != nil { return nil, err } - tp := sdktrace.NewTracerProvider(sdktrace.WithBatcher(exp)) - if exp.o.config != nil { - tp.ApplyConfig(*exp.o.config) + tp := sdktrace.NewTracerProvider(sdktrace.WithBatcher(exporter)) + if exporter.o.config != nil { + tp.ApplyConfig(*exporter.o.config) } return tp, err @@ -166,19 +167,21 @@ func (e *Exporter) ExportSpans(ctx context.Context, batch []*export.SpanData) er if err != nil { return e.errf("request to %s failed: %v", e.url, err) } - e.logf("zipkin responded with status %d", resp.StatusCode) + defer resp.Body.Close() - _, err = ioutil.ReadAll(resp.Body) + // Zipkin API returns a 202 on success and the content of the body isn't interesting + // but it is still being read because according to https://golang.org/pkg/net/http/#Response + // > The default HTTP client's Transport may not reuse HTTP/1.x "keep-alive" TCP connections + // > if the Body is not read to completion and closed. + _, err = io.Copy(ioutil.Discard, resp.Body) if err != nil { - // Best effort to clean up here. - resp.Body.Close() return e.errf("failed to read response body: %v", err) } - err = resp.Body.Close() - if err != nil { - return e.errf("failed to close response body: %v", err) + if resp.StatusCode != http.StatusAccepted { + return e.errf("failed to send spans to zipkin server with status %d", resp.StatusCode) } + return nil } diff --git a/exporters/trace/zipkin/zipkin_test.go b/exporters/trace/zipkin/zipkin_test.go index da0b3b4a994..e469f5d88e6 100644 --- a/exporters/trace/zipkin/zipkin_test.go +++ b/exporters/trace/zipkin/zipkin_test.go @@ -192,6 +192,7 @@ func (c *mockZipkinCollector) handler(w http.ResponseWriter, r *http.Request) { c.lock.Lock() defer c.lock.Unlock() c.models = append(c.models, models...) + w.WriteHeader(http.StatusAccepted) } func (c *mockZipkinCollector) Close() { @@ -340,9 +341,8 @@ func TestExportSpans(t *testing.T) { ctx := context.Background() require.Len(t, ls.Messages, 0) require.NoError(t, exporter.ExportSpans(ctx, spans[0:1])) - require.Len(t, ls.Messages, 2) + require.Len(t, ls.Messages, 1) require.Contains(t, ls.Messages[0], "send a POST request") - require.Contains(t, ls.Messages[1], "zipkin responded") ls.Messages = nil require.NoError(t, exporter.ExportSpans(ctx, nil)) require.Len(t, ls.Messages, 1) @@ -350,7 +350,6 @@ func TestExportSpans(t *testing.T) { ls.Messages = nil require.NoError(t, exporter.ExportSpans(ctx, spans[1:2])) require.Contains(t, ls.Messages[0], "send a POST request") - require.Contains(t, ls.Messages[1], "zipkin responded") checkFunc := func() bool { return collector.ModelsLen() == len(models) }