fix(span): correctly parse multi-segment trace targets in single-arg view#1264
fix(span): correctly parse multi-segment trace targets in single-arg view#1264sentry[bot] wants to merge 1 commit into
Conversation
|
Codecov Results 📊✅ Patch coverage is 84.62%. Project has 5477 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 81.67% 81.67% —%
==========================================
Files 425 425 —
Lines 29868 29875 +7
Branches 19398 19398 —
==========================================
+ Hits 24393 24398 +5
- Misses 5475 5477 +2
- Partials 2033 2034 +1Generated by Codecov Action |
| // Multi-segment form: org/project/trace-id/span-id — defer so that | ||
| // parseTraceTargetWithRecovery resolves the full trace target. | ||
| log.warn( | ||
| `Interpreting '${first}' as <trace-target>/<span-id>. ` + | ||
| `Use separate arguments: sentry span view ${rawLeft} ${right}` | ||
| ); | ||
| return { | ||
| kind: "deferred", | ||
| rawTraceArg: rawLeft, | ||
| rawSpanIds: [right], | ||
| }; |
There was a problem hiding this comment.
Bug: The new multi-slash handling logic incorrectly parses arguments like org/project/span-id, leading to a confusing error about an invalid trace ID instead of a missing span ID.
Severity: MEDIUM
Suggested Fix
Add validation to the hasMultipleSlashes branch to ensure that arguments like org/project/span-id are parsed correctly. The logic should be similar to the single-slash branch, ensuring that the components are correctly identified as organization, project, and span ID, and provide a clear error if the format is invalid.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/commands/span/view.ts#L121-L132
Potential issue: The logic for handling arguments with multiple slashes, such as
`org/project/span-id`, incorrectly splits the input. It treats `org/project` as the
trace target and `span-id` as the span ID. This causes `parseTraceTargetWithRecovery` to
attempt to validate `project` as a trace ID, which fails and produces a confusing
`ValidationError: Invalid trace ID "project"`. This is a regression from the previous
behavior, which would have correctly identified that a span ID was missing and provided
a clearer error message. The new `hasMultipleSlashes` branch lacks the necessary
validation to prevent this incorrect parsing.
Did we get this right? 👍 / 👎 to inform future reviews.
This PR addresses issue CLI-1BD, where
sentry span viewfailed with "ContextError: Span ID is required." when a single argument was provided in the formatorg/project/trace-id/span-id.Problem:
The
parsePositionalArgsfunction's auto-detection logic for single arguments previously only triggered if there was exactly one slash (e.g.,trace-id/span-id). Inputs with multiple slashes (e.g.,org/project/trace-id/span-id) were not matched by this logic. This resulted inrawSpanIdsbeing empty, leading to theContextError.Solution:
The auto-detection logic in
parsePositionalArgshas been updated:lastIndexOf('/')to find the final slash in the single argument.org/project/trace-id/span-id), the left part (org/project/trace-id) is now passed as adeferredrawTraceArg. This allowsparseTraceTargetWithRecoveryto correctly resolve the multi-segment trace target.trace-id/span-id), the originalresolvedpath is maintained, ensuring the left part is a valid trace ID. This preserves existing behavior and test expectations for simpler cases.This change ensures that
sentry span viewcan correctly interpret fully-qualified trace targets provided as a single argument.Fixes CLI-1BD
Comment
@sentry <feedback>on this PR to have Autofix iterate on the changes.