[exporter/opensearch] Retry transient failures instead of dropping them as permanent

[alliasgher]

Description

The OpenSearch exporter's bulk-indexer OnError callback (onIndexerError) wrapped every indexer-level error as consumererror.NewPermanent. Indexer-level errors are transport/flush failures (connection refused, timeout, DNS), which are transient. Marking them permanent made exporterhelper drop the whole batch even with retry_on_failure fully configured, so telemetry was silently lost whenever OpenSearch was briefly unreachable.

This surfaces indexer-level errors as retryable (a plain error, so joinedError() is not permanent and exporterhelper resends the batch). In processItemFailure, a per-item net.OpError (transport) is now treated as retryable as well, while encoding errors and non-recoverable OpenSearch responses (bad documents) stay permanent. The same fix is applied to both the log and trace bulk indexers.

Link to tracking issue

Fixes #49208

Testing

Added TestOnIndexerErrorIsRetryable and TestProcessItemFailureTransportErrorRetryable (and the trace equivalents), asserting via consumererror.IsPermanent that transport failures are retryable and that encoding errors remain permanent. Existing bulk-indexer tests still pass.

Documentation

No documentation changes needed.

[opentelemetry-pr-dashboard]

Pull request dashboard status

Status last refreshed: 2026-07-21 07:53:28 UTC.

• Waiting on: Author
• Next step: Address or respond to 4 review feedback items:
  • Inline threads: 1, 2, 3, 4
  • For each item, link to the commit that addresses it, explain why no change is needed, or ask a follow-up question.

This automated status or its linked feedback items may be incorrect. If something looks wrong, report it with the result you expected.

[kylehounslow]

This only retries *net.OpError. Should a more exhaustive list (e.g. context.DeadlineExceeded, context.Canceled) be covered? Or, assuming we have a safe retry/back-off configuration, should we consider defaulting to retryable and reserve Permanent for errors we can positively ID as non-recoverable?

[kylehounslow]

On a flush failure, doesn't opensearchutil fire both OnFailure (per item, via handleBulkError) and OnError with the same error? Since IsPermanent is errors.As over the joined error, wouldn't this Permanent branch override the retryable onIndexerError fix and drop the batch anyway? See code here and here

[kylehounslow]

Could we drive a real flush via httptest and assert on the joined error, rather than hand-feeding *net.OpError? That would exercise the actual error types the transport produces. Something like:

func TestProcessItemFailureTransportRetryable(t *testing.T) {
	tests := []struct {
		name    string
		handler http.HandlerFunc // nil => point at a dead port
	}{
		{"connection refused", nil},
		{"connection drop mid-request", func(w http.ResponseWriter, _ *http.Request) {
			conn, _, _ := w.(http.Hijacker).Hijack()
			conn.Close()
		}},
		{"reset after read", func(w http.ResponseWriter, r *http.Request) {
			io.Copy(io.Discard, r.Body)
			conn, _, _ := w.(http.Hijacker).Hijack()
			if tcp, ok := conn.(*net.TCPConn); ok {
				tcp.SetLinger(0) // force RST
			}
			conn.Close()
		}},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			endpoint := "http://127.0.0.1:1" // dead port
			if tt.handler != nil {
				srv := httptest.NewServer(tt.handler)
				defer srv.Close()
				endpoint = srv.URL
			}
			client, _ := newOpenSearchClient(endpoint, http.DefaultClient, zap.NewNop())
			idx := newLogBulkIndexer("create", model, "")
			require.NoError(t, idx.start(client))
			idx.submit(context.Background(), logs, ir, cfg, time.Now())
			idx.close(context.Background())

			err := idx.joinedError()
			require.Error(t, err)
			require.False(t, consumererror.IsPermanent(err),
				"transient flush failure must be retryable, else retry_on_failure drops the batch")
		})
	}
}

[kylehounslow]

This looks like it also fixes #45147 and is closely related to #45120. Worth adding them to issues: so they get closed/linked