Tryout otlp collector fix for retryable http errors #3515
Conversation
|
|
|
Waiting to see if the original PR can be merged upstream and then we just have to vendor the main collector repository instead of the fork. |
jesusvazquez
requested review from
joe-elliott,
annanay25,
mdisibio,
mapno,
yvrhdn,
zalegrala,
electron0zero,
ie-pham and
stoewer
as code owners
| require.NoError(t, err) | ||
| fmt.Println(string(bodyBytes)) | ||
|
|
||
| assert.Equal(t, http.StatusTooManyRequests, resp.StatusCode) |
There was a problem hiding this comment.
Check my understanding, but we are sending multiple traces in one reqeust, and the config specifies a limit of one trace per user. Should this fail with "TooManyRequests" since we are only making one request?
There was a problem hiding this comment.
The config limits fixture file used in this tests specifies the following https://github.com/mapno/tempo/blob/70bf4e253df931f68e747b267c496adaa8e77f6a/integration/e2e/config-limits.yaml#L20
overrides:
defaults:
ingestion:
max_traces_per_user: 1
rate_limit_bytes: 500
burst_size_bytes: 500
global:
max_bytes_per_trace: 130
The test is batching up 10 traces
https://github.com/mapno/tempo/blob/5faf0f091584b25e43f57fdcb27e5f2da7cc342a/integration/e2e/limits_test.go#L146
So the quota is being hit and the status code returned is 429 which is known as Resource Exhausted but in go its also TooManyRequests https://github.com/golang/go/blob/master/src/net/http/status.go#L62
Signed-off-by: Jesus Vazquez <jesus.vazquez@grafana.com>
jesusvazquez
force-pushed
the
jvp/otlp-http-500s-and-collector-fix
branch
from
661c3f3
to
c1a61d9
Compare
What this PR does:
Tries out a collector version that includes open-telemetry/opentelemetry-collector#9357 and adds two e2e tests to make sure that the receiver replies with retryable errors.
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]