[receiver/sapm] - follow receiver contract

.chloggen/sapm-receiver-contract.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: enhancement
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: sapmreceiver
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Respond 503 on non-permanent and 400 on permanent errors 
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [35300]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

internal/coreinternal/errorutil/http.go

@@ -0,0 +1,23 @@
+// Copyright The OpenTelemetry Authors
+// SPDX-License-Identifier: Apache-2.0
+
+package errorutil // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/errorutil"
+
+import (
+	"net/http"
+
+	"go.opentelemetry.io/collector/consumer/consumererror"
+)
+
+func HTTPError(w http.ResponseWriter, err error) {
+	if err == nil {
+		return
+	}
+	// non-retryable status
+	status := http.StatusBadRequest
+	if !consumererror.IsPermanent(err) {
+		// retryable status
+		status = http.StatusServiceUnavailable
+	}
+	http.Error(w, err.Error(), status)
+}

receiver/sapmreceiver/go.mod

@@ -7,6 +7,7 @@ require (
 	github.com/jaegertracing/jaeger v1.61.0
 	github.com/klauspost/compress v1.17.10
 	github.com/open-telemetry/opentelemetry-collector-contrib/internal/common v0.111.0
+	github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.111.0
 	github.com/open-telemetry/opentelemetry-collector-contrib/internal/splunk v0.111.0
 	github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger v0.111.0
 	github.com/signalfx/sapm-proto v0.14.0
@@ -47,7 +48,6 @@ require (
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
-	github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.111.0 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/rs/cors v1.11.1 // indirect
 	go.opentelemetry.io/collector/client v1.17.0 // indirect

receiver/sapmreceiver/trace_receiver.go

@@ -22,6 +22,7 @@ import (
 	"go.opentelemetry.io/collector/receiver"
 	"go.opentelemetry.io/collector/receiver/receiverhelper"
 
+	"github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/errorutil"
 	"github.com/open-telemetry/opentelemetry-collector-contrib/internal/splunk"
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/jaeger"
 )
@@ -91,8 +92,7 @@ func (sr *sapmReceiver) HTTPHandlerFunc(rw http.ResponseWriter, req *http.Reques
 	// handle the request payload
 	err := sr.handleRequest(req)
 	if err != nil {
-		// TODO account for this error (throttled logging or metrics)
-		rw.WriteHeader(http.StatusBadRequest)
+		errorutil.HTTPError(rw, err)
 		return
 	}
 

receiver/sapmreceiver/trace_receiver_test.go

@@ -8,6 +8,7 @@ import (
 	"compress/gzip"
 	"context"
 	"encoding/binary"
+	"errors"
 	"fmt"
 	"net/http"
 	"testing"
@@ -23,6 +24,8 @@ import (
 	"go.opentelemetry.io/collector/component/componentstatus"
 	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configtls"
+	"go.opentelemetry.io/collector/consumer"
+	"go.opentelemetry.io/collector/consumer/consumererror"
 	"go.opentelemetry.io/collector/consumer/consumertest"
 	"go.opentelemetry.io/collector/pdata/pcommon"
 	"go.opentelemetry.io/collector/pdata/ptrace"
@@ -243,7 +246,7 @@ func compressZstd(reqBytes []byte) ([]byte, error) {
 	return buff.Bytes(), nil
 }
 
-func setupReceiver(t *testing.T, config *Config, sink *consumertest.TracesSink) receiver.Traces {
+func setupReceiver(t *testing.T, config *Config, sink consumer.Traces) receiver.Traces {
 	params := receivertest.NewNopSettings()
 	sr, err := newReceiver(params, config, sink)
 	assert.NoError(t, err, "should not have failed to create the SAPM receiver")
@@ -432,6 +435,45 @@ func TestAccessTokenPassthrough(t *testing.T) {
 	}
 }
 
+func TestStatusCode(t *testing.T) {
+	tlsAddress := testutil.GetAvailableLocalAddress(t)
+
+	tests := []struct {
+		name           string
+		err            error
+		expectedStatus int
+	}{
+		{
+			name:           "non-permanent error",
+			err:            errors.New("non-permanenet error"),
+			expectedStatus: http.StatusServiceUnavailable,
+		},
+		{
+			name:           "permanent error",
+			err:            consumererror.NewPermanent(errors.New("non-permanenet error")),
+			expectedStatus: http.StatusBadRequest,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			config := &Config{
+				ServerConfig: confighttp.ServerConfig{
+					Endpoint: tlsAddress,
+				},
+			}
+			sr := setupReceiver(t, config, consumertest.NewErr(test.err))
+			sapm := &splunksapm.PostSpansRequest{
+				Batches: []*model.Batch{grpcFixture(time.Now().UTC())},
+			}
+			var resp *http.Response
+			resp, err := sendSapm(config.Endpoint, sapm, "", false, "")
+			require.NoErrorf(t, err, "should not have failed when sending sapm %v", err)
+			assert.Equal(t, test.expectedStatus, resp.StatusCode)
+			require.NoError(t, sr.Shutdown(context.Background()))
+		})
+	}
+}
+
 var _ componentstatus.Reporter = (*nopHost)(nil)
 
 type nopHost struct {