[exporter/prometheusremotewriteexporter] Prometheus remote write exporter does not retry on 5xx

[rapphil]

Component(s)

exporter/prometheusremotewrite

What happened?

Description

The prometheusremotewriteexporter is failing the conformance tests of prometheus.

--- FAIL: TestRemoteWrite (24.70s)
    --- FAIL: TestRemoteWrite/otelcollector (0.01s)
        --- PASS: TestRemoteWrite/otelcollector/Up (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/Counter (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/JobLabel (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/Histogram (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/Staleness (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/Invalid (10.03s)
        --- PASS: TestRemoteWrite/otelcollector/EmptyLabels (10.03s)
        --- PASS: TestRemoteWrite/otelcollector/Headers (10.03s)
        --- PASS: TestRemoteWrite/otelcollector/HonorLabels (10.05s)
        --- PASS: TestRemoteWrite/otelcollector/NameLabel (10.05s)
        --- PASS: TestRemoteWrite/otelcollector/Timestamp (10.05s)
        --- PASS: TestRemoteWrite/otelcollector/Summary (10.06s)
        --- PASS: TestRemoteWrite/otelcollector/SortedLabels (10.01s)
        --- FAIL: TestRemoteWrite/otelcollector/Retries500 (10.01s)
        --- PASS: TestRemoteWrite/otelcollector/RepeatedLabels (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/Retries400 (10.01s)
        --- PASS: TestRemoteWrite/otelcollector/InstanceLabel (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/Gauge (10.02s)
        --- PASS: TestRemoteWrite/otelcollector/Ordering (14.66s)
FAIL
FAIL    github.com/prometheus/compliance/remote_write   25.036s
FAIL

Steps to Reproduce

[email protected]:prometheus/compliance.git
cd compliance/remote_write_sender
go test --tags=compliance -run "TestRemoteWrite/otelcollector/.+" -v ./

Expected Result

The exporter should retry on 5xx errors.

Actual Result

The exporter does not retry 5xx. It is treating it in the same way as 4xx.

Collector version

this regression was inserted starting with v0.45.0

Environment information

RHEL based.

OpenTelemetry Collector configuration

NA

Log output

NA

Additional context

NA

[github-actions]

Pinging code owners:

• exporter/prometheusremotewrite: @Aneurysm9

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[rapphil]

I would like to propose that we add a retry in the moment a batch of metrics is exported: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/prometheusremotewriteexporter/exporter.go#L250

This would increase the risk of out of order samples, because the component can operate with multiple workers that export the batches in parallel: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/prometheusremotewriteexporter/exporter.go#L202

To eliminate the risk of out of order samples, I also propose that we change how we submit data in parallel. Instead of just submitting the batches in parallel, we should try to make sure that a single time series is always submitted to prometheus sequentially. The idea is that you can only submit samples for different time series in parallel. Samples for the same time series should always be written sequentially.

To achieve this I propose that:

• We create one queue per worker. Each worker will consume from its own queue.
• We partition the data that will be submitted to each of the queues so that a single time series will only be submitted to a specific queue. This can be a hash(labelset) modulo NUMBER_QUEUES.
• each worker can retry independently on a 5xx status code.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• exporter/prometheusremotewrite: @Aneurysm9 @rapphil

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• exporter/prometheusremotewrite: @Aneurysm9 @rapphil

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[rapphil]

There is still work being done for this issue in #23842

[rapphil]

/label -stale

[vasireddy99]

The PR is merged - #23842 and this issue can be closed .

[rapphil]

Closing as the PR was merged.