Skip to content

Commit

Permalink
fix: Fix OTLP http responses (#1010)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?

Refinery was not returning the correct OTLP payload and not always using
the correct `Content-Type` in the response.

- closes #1008

## Short description of the changes

This PR attempts to fix the OTLP response while leaving other protocols
unaffected.

- Creates a new error handler method for the OTLP endpoint
- Ensures a `ExportTraceServiceResponse` object is returned for success
and a grpc `Status` object is used for failure.
- Changes the "bad API key" response code from 400 to 401.

---------

Co-authored-by: Robb Kidd <[email protected]>
  • Loading branch information
TylerHelmuth and robbkidd authored Feb 28, 2024
1 parent e1186ea commit 6f5aef9
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 10 deletions.
2 changes: 1 addition & 1 deletion app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func TestAppIntegrationWithUnauthorizedKey(t *testing.T) {

resp, err := http.DefaultTransport.RoundTrip(req)
assert.NoError(t, err)
assert.Equal(t, 400, resp.StatusCode)
assert.Equal(t, 401, resp.StatusCode)
data, err := io.ReadAll(resp.Body)
resp.Body.Close()
assert.NoError(t, err)
Expand Down
23 changes: 23 additions & 0 deletions route/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package route

import (
"fmt"
"io"
"net/http"
"runtime/debug"

Expand Down Expand Up @@ -73,3 +74,25 @@ func (r *Router) handlerReturnWithError(w http.ResponseWriter, he handlerError,

w.Write(jsonErrMsg)
}

func (r *Router) handleOTLPFailureResponse(w http.ResponseWriter, req *http.Request, otlpErr husky.OTLPError) {
r.Logger.Error().Logf(otlpErr.Error())

if otlpErr != husky.ErrInvalidContentType {
err := husky.WriteOtlpHttpFailureResponse(w, req, otlpErr)
if err != nil {
// If we made it here we had a problem writing an OTLP HTTP response
resp := fmt.Sprintf("failed to write otlp http response, %v", err.Error())
r.Logger.Error().Logf(resp)
w.Header().Set("Content-Type", "text/plain")
w.WriteHeader(http.StatusInternalServerError)
_, _ = io.WriteString(w, resp)
}
} else {
// Since we have an invalid content type for an OTLP HTTP response we choose to use to text/plain
w.Header().Set("Content-Type", "text/plain")
w.WriteHeader(otlpErr.HTTPStatusCode)
_, _ = io.WriteString(w, otlpErr.Message)
}

}
14 changes: 8 additions & 6 deletions route/otlp_trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,31 @@ func (r *Router) postOTLP(w http.ResponseWriter, req *http.Request) {
ri := huskyotlp.GetRequestInfoFromHttpHeaders(req.Header)

if !r.Config.IsAPIKeyValid(ri.ApiKey) {
err := fmt.Errorf("api key %s not found in list of authorized keys", ri.ApiKey)
r.handlerReturnWithError(w, ErrAuthNeeded, err)
r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: fmt.Sprintf("api key %s not found in list of authorized keys", ri.ApiKey), HTTPStatusCode: http.StatusUnauthorized})
return
}

if err := ri.ValidateTracesHeaders(); err != nil {
if errors.Is(err, huskyotlp.ErrInvalidContentType) {
r.handlerReturnWithError(w, ErrInvalidContentType, err)
r.handleOTLPFailureResponse(w, req, huskyotlp.ErrInvalidContentType)
} else {
r.handlerReturnWithError(w, ErrAuthNeeded, err)
r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusUnauthorized})
}
return
}

result, err := huskyotlp.TranslateTraceRequestFromReader(req.Body, ri)
if err != nil {
r.handlerReturnWithError(w, ErrUpstreamFailed, err)
r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusInternalServerError})
return
}

if err := processTraceRequest(req.Context(), r, result.Batches, ri.ApiKey); err != nil {
r.handlerReturnWithError(w, ErrUpstreamFailed, err)
r.handleOTLPFailureResponse(w, req, huskyotlp.OTLPError{Message: err.Error(), HTTPStatusCode: http.StatusInternalServerError})
return
}

_ = huskyotlp.WriteOtlpHttpTraceSuccessResponse(w, req)
}

type TraceServer struct {
Expand Down
4 changes: 2 additions & 2 deletions route/otlp_trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func TestOTLPHandler(t *testing.T) {
w := httptest.NewRecorder()
router.postOTLP(w, request)
assert.Equal(t, w.Code, http.StatusOK)
assert.Equal(t, "", w.Body.String())
assert.Equal(t, "{}", w.Body.String())

assert.Equal(t, 2, len(mockTransmission.Events))
mockTransmission.Flush()
Expand Down Expand Up @@ -409,7 +409,7 @@ func TestOTLPHandler(t *testing.T) {

w := httptest.NewRecorder()
router.postOTLP(w, request)
assert.Equal(t, http.StatusBadRequest, w.Code)
assert.Equal(t, http.StatusUnauthorized, w.Code)
assert.Contains(t, w.Body.String(), "not found in list of authorized keys")

assert.Equal(t, 0, len(mockTransmission.Events))
Expand Down
1 change: 0 additions & 1 deletion route/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,6 @@ func (r *Router) Watch(req *grpc_health_v1.HealthCheckRequest, server grpc_healt
func (r *Router) AddOTLPMuxxer(muxxer *mux.Router) {
// require an auth header for OTLP requests
otlpMuxxer := muxxer.PathPrefix("/v1/").Methods("POST").Subrouter()
otlpMuxxer.Use(r.apiKeyChecker)

// handle OTLP trace requests
otlpMuxxer.HandleFunc("/traces", r.postOTLP).Name("otlp")
Expand Down

0 comments on commit 6f5aef9

Please sign in to comment.