Repository navigation
Complete spans for every retry attempt, add error ignoring options - #12
Merged
Merged
Conversation
Resty re-runs OnBeforeRequest on every retry attempt, but only the span of the final attempt was ever completed: attempts that failed before a response arrived were left dangling (issue #9), and OnError recorded the error on an already-ended span when a response was present. The middleware now tracks the current attempt span in the request context and closes the previous attempt's span when a retry starts. Response and error handling operate only on that stored span, so requests skipped by the skipper no longer end a caller-provided span. Also adds WithIgnoredErrors and WithErrorIgnorer (issue #7), so benign errors like resty.ErrAutoRedirectDisabled no longer mark spans as erroneous, and updates request/response attributes to the current stable HTTP semantic conventions (http.request.method, url.full, url.path, server.address/port, http.response.status_code) replacing the ancient semconv v1.4.0 and httpconv v1.13.0 conversion helpers. Fixes #9 Fixes #7
dubonzi
force-pushed
the
fix/retry-spans-error-ignorer
branch
from
October 12, 2026 12:37
07fe585 to
795c7d8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OnBeforeRequeston every retry attempt, but only the final attempt's span was ever completed. Attempts that failed before a response arrived (transport errors) were left dangling, andOnErrorrecorded the error on an already-ended span. The middleware now tracks the current attempt's span in the request context, chains each attempt to the previous one, and closes the previous attempt's span when a retry starts. A regression test reproduces the dangling-span behavior onmainand passes with this change.WithIgnoredErrors(errs ...error)option (matched witherrors.Is, covering sentinels likeresty.ErrAutoRedirectDisabled) andWithErrorIgnorer(f func(error) bool)predicate option. Ignored errors no longer record an error event or set error status.WithSkipperactive and a caller-provided span in the request context, the middleware would end the caller's span.semconv/v1.4.0andhttpconv v1.13.0conversion helpers with the current stable HTTP attributes fromsemconv/v1.43.0(http.request.method,url.full,url.path,server.address,server.port,http.request.body.size,http.response.status_code,http.response.body.size).resty.urlbehavior is unchanged. Note this is an observable change for consumers matching on the old attribute names.Tests
go test ./... -count=1 -racepasses, including new tests for: retry attempt span completion and parent-child chaining, all-attempts-fail error recording, ignored errors (list and predicate), skipper not ending caller spans, andWithHideURLredaction.