Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xray client: return an error if the HTTP request failed #5718

Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The double setup in `go.opentelemetry.io/contrib/instrumentation/net/http/httptrace/otelhttptrace/example` that caused duplicate traces. (#5564)
- Out-of-bounds panic in case of invalid span ID in `go.opentelemetry.io/contrib/propagators/b3`. (#5754)
- Do not panic if a zero-value `SpanProcessor` is used from `go.opentelemetry.io/contrib/processors/baggage/baggagetrace`. (#5811)
- Non-200 HTTP status codes when retrieving sampling rules in `go.opentelemetry.io/contrib/samplers/aws/xray` now return an error. (#5718)
jaedle marked this conversation as resolved.
Show resolved Hide resolved

### Deprecated

Expand Down
18 changes: 13 additions & 5 deletions samplers/aws/xray/internal/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,22 @@

req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.samplingRulesURL, body)
if err != nil {
return nil, fmt.Errorf("xray client: failed to create http request: %w", err)
return nil, fmt.Errorf("unable to retrieve sampling rules, error on http request: %w", err)

Check warning on line 133 in samplers/aws/xray/internal/client.go

View check run for this annotation

Codecov / codecov/patch

samplers/aws/xray/internal/client.go#L133

Added line #L133 was not covered by tests
}

output, err := c.httpClient.Do(req)
if err != nil {
return nil, fmt.Errorf("xray client: unable to retrieve sampling settings: %w", err)
return nil, fmt.Errorf("xray client: unable to retrieve sampling rules, error on http request: %w", err)
}
defer output.Body.Close()

if output.StatusCode != http.StatusOK {
jaedle marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("xray client: unable to retrieve sampling rules, expected response status code 200, got: %d", output.StatusCode)
}

var samplingRulesOutput *getSamplingRulesOutput
if err := json.NewDecoder(output.Body).Decode(&samplingRulesOutput); err != nil {
return nil, fmt.Errorf("xray client: unable to unmarshal the response body: %w", err)
return nil, fmt.Errorf("xray client: unable to retrieve sampling rules, unable to unmarshal the response body: %w", err)
}

return samplingRulesOutput, nil
Expand All @@ -166,13 +170,17 @@

output, err := c.httpClient.Do(req)
if err != nil {
return nil, fmt.Errorf("xray client: unable to retrieve sampling settings: %w", err)
return nil, fmt.Errorf("xray client: unable to retrieve sampling targets, error on http request: %w", err)
}
defer output.Body.Close()

if output.StatusCode != http.StatusOK {
return nil, fmt.Errorf("xray client: unable to retrieve sampling targets, expected response status code 200, got: %d", output.StatusCode)
}

var samplingTargetsOutput *getSamplingTargetsOutput
if err := json.NewDecoder(output.Body).Decode(&samplingTargetsOutput); err != nil {
return nil, fmt.Errorf("xray client: unable to unmarshal the response body: %w", err)
return nil, fmt.Errorf("xray client: unable to retrieve sampling targets, unable to unmarshal the response body: %w", err)
}

return samplingTargetsOutput, nil
Expand Down
79 changes: 63 additions & 16 deletions samplers/aws/xray/internal/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package internal

import (
"context"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
Expand All @@ -15,7 +16,12 @@ import (
)

func createTestClient(t *testing.T, body []byte) *xrayClient {
return createTestClientWithStatusCode(t, http.StatusOK, body)
}

func createTestClientWithStatusCode(t *testing.T, status int, body []byte) *xrayClient {
testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, _ *http.Request) {
res.WriteHeader(status)
_, err := res.Write(body)
require.NoError(t, err)
}))
Expand All @@ -26,7 +32,6 @@ func createTestClient(t *testing.T, body []byte) *xrayClient {

client, err := newClient(*u)
require.NoError(t, err)

return client
}

Expand Down Expand Up @@ -222,22 +227,11 @@ func TestGetSamplingTargetsMissingValues(t *testing.T) {

client := createTestClient(t, body)

samplingTragets, err := client.getSamplingTargets(ctx, nil)
samplingTargets, err := client.getSamplingTargets(ctx, nil)
require.NoError(t, err)

assert.Nil(t, samplingTragets.SamplingTargetDocuments[0].Interval)
assert.Nil(t, samplingTragets.SamplingTargetDocuments[0].ReservoirQuota)
}

func TestNilContext(t *testing.T) {
client := createTestClient(t, []byte(``))
samplingRulesOutput, err := client.getSamplingRules(context.TODO())
require.Error(t, err)
require.Nil(t, samplingRulesOutput)

samplingTargetsOutput, err := client.getSamplingTargets(context.TODO(), nil)
require.Error(t, err)
require.Nil(t, samplingTargetsOutput)
assert.Nil(t, samplingTargets.SamplingTargetDocuments[0].Interval)
assert.Nil(t, samplingTargets.SamplingTargetDocuments[0].ReservoirQuota)
}

func TestNewClient(t *testing.T) {
Expand All @@ -258,6 +252,59 @@ func TestEndpointIsNotReachable(t *testing.T) {
client, err := newClient(*endpoint)
require.NoError(t, err)

_, err = client.getSamplingRules(context.Background())
actualRules, err := client.getSamplingRules(context.Background())
assert.Error(t, err)
assert.ErrorContains(t, err, "xray client: unable to retrieve sampling rules, error on http request: ")
assert.Nil(t, actualRules)

actualTargets, err := client.getSamplingTargets(context.Background(), nil)
assert.Error(t, err)
assert.ErrorContains(t, err, "xray client: unable to retrieve sampling targets, error on http request: ")
assert.Nil(t, actualTargets)
}

func TestRespondsWithErrorStatusCode(t *testing.T) {
client := createTestClientWithStatusCode(t, http.StatusForbidden, []byte("{}"))

actualRules, err := client.getSamplingRules(context.Background())
assert.Error(t, err)
assert.EqualError(t, err, fmt.Sprintf("xray client: unable to retrieve sampling rules, expected response status code 200, got: %d", http.StatusForbidden))
assert.Nil(t, actualRules)

actualTargets, err := client.getSamplingTargets(context.Background(), nil)
assert.Error(t, err)
assert.EqualError(t, err, fmt.Sprintf("xray client: unable to retrieve sampling targets, expected response status code 200, got: %d", http.StatusForbidden))
assert.Nil(t, actualTargets)
}

func TestInvalidResponseBody(t *testing.T) {
type scenarios struct {
name string
response string
}
for _, scenario := range []scenarios{
{
name: "empty response",
response: "",
},
{
name: "malformed json",
response: "",
},
} {
t.Run(scenario.name, func(t *testing.T) {
client := createTestClient(t, []byte(scenario.response))

actualRules, err := client.getSamplingRules(context.TODO())

assert.Error(t, err)
assert.Nil(t, actualRules)
assert.ErrorContains(t, err, "xray client: unable to retrieve sampling rules, unable to unmarshal the response body:"+scenario.response)

actualTargets, err := client.getSamplingTargets(context.TODO(), nil)
assert.Error(t, err)
assert.Nil(t, actualTargets)
assert.ErrorContains(t, err, "xray client: unable to retrieve sampling targets, unable to unmarshal the response body: "+scenario.response)
})
}
}