fix(go/tracing): mark only originating span as failure source - #6047
Open
ktsoator wants to merge 2 commits into
Open
fix(go/tracing): mark only originating span as failure source#6047ktsoator wants to merge 2 commits into
ktsoator wants to merge 2 commits into
Conversation
Track handled errors as they propagate through nested Genkit spans so only the span where an error originates receives genkit:isFailureSource. Parent spans remain in the error state and continue to record OpenTelemetry error events and status. Keep the internal marker out of OpenTelemetry exception types and unwrap it at the outermost Genkit span to preserve the application's original error identity and errors.Is/errors.As behavior. Add regression coverage for nested span attributes, error states, error identity, and marker unwrapping. Fixes genkit-ai#5153
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces an unwrapMarkedError helper function to preserve the original error identity in telemetry, updates RunInNewSpan to correctly mark and unwrap errors, and enhances testing around failure sources and error unwrapping. Feedback on these changes includes adding a nil check during type assertion in unwrapMarkedError to prevent potential nil pointer dereferences, and checking if err != nil before unwrapping in RunInNewSpan to avoid unnecessary operations.
Handle typed-nil marked errors without dereferencing them and avoid marker unwrapping on successful root spans. Add regression coverage for the typed-nil case.
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
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
genkit:isFailureSourceProblem
RunInNewSpancurrently setsIsFailureSourceon every span that observes a returned error. As the same error propagates through nested actions and flows, each parent span therefore exportsgenkit:isFailureSource=true, which obscures the actual source and can overcount failures in monitoring.The tracing package already contains
markedError,markErrorAsHandled, andisErrorAlreadyMarked, but the marker was never applied inRunInNewSpan.Implementation
The originating span now marks and wraps an unhandled error before returning it. Parent spans recognize the marker and omit
genkit:isFailureSource, while still recording their normal OpenTelemetry error event and error status.The internal wrapper is removed when recording the application error in OpenTelemetry and when the error exits the outermost Genkit span. This keeps
exception.type, direct sentinel comparisons, anderrors.Is/errors.Asbehavior compatible with the original error.Testing
Added regression coverage that verifies:
genkit:isFailureSource=trueerrors.Isbehavior are preservedCommands run:
All passed.
Fixes #5153
Checklist