feat(core)!: Remove kind for spans, move to sentry.kind attribute#22528
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| /** An op for the span. This is a categorization for spans. */ | ||
| op?: string; | ||
|
|
||
| /** |
There was a problem hiding this comment.
Public API broken without deprecation
High Severity
This removes public surface without a deprecation path in a v10 minor: the kind option on StartSpanOptions, the SPAN_KIND / SpanKindValue exports from @sentry/core, and getSpanKind from @sentry/opentelemetry. The review rules require deprecation notices for public API removals and signature changes. Flagged because it was mentioned in this rules file.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 973aafc. Configure here.
There was a problem hiding this comment.
need to add deprecation in follow up
size-limit report 📦
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
There are 5 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0863e20. Configure here.
| hook: 'preprocessSpan', | ||
| callback: (streamedSpanJSON: StreamedSpanJSON, hint?: { spanKind?: number }) => void, | ||
| ): () => void; | ||
| public on(hook: 'preprocessSpan', callback: (streamedSpanJSON: StreamedSpanJSON) => void): () => void; |
There was a problem hiding this comment.
preprocessSpan hint removed undeprecated
Medium Severity
The public preprocessSpan hook no longer accepts the { spanKind } hint argument. External subscribers typing or depending on that second parameter break without a deprecation path. Flagged because it was mentioned in the PR review rules file.
Additional Locations (1)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 0863e20. Configure here.
There was a problem hiding this comment.
that's fine IMHO, since nothing else is in the hint
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const opParts = ['http']; | ||
| const kind = attributes[SENTRY_KIND]; | ||
|
|
||
| switch (kind) { | ||
| case SpanKind.CLIENT: | ||
| case 'client': | ||
| opParts.push('client'); | ||
| break; | ||
| case SpanKind.SERVER: | ||
| case 'server': | ||
| opParts.push('server'); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Bug: The code incorrectly reads the span kind from attributes[SENTRY_KIND], which is not available for standard OTel spans, leading to incorrect op and transaction names.
Severity: HIGH
Suggested Fix
Modify parseSpanDescription to get the span kind from the ReadableSpan.kind property directly, rather than relying on the SENTRY_KIND attribute which is populated later. This ensures the correct kind is used for determining the op and inferring the description.
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: packages/opentelemetry/src/utils/parseSpanDescription.ts#L149-L165
Potential issue: The function `descriptionForHttpMethod` now reads the span kind from
`attributes[SENTRY_KIND]`. However, for spans from standard OpenTelemetry
instrumentations like `@opentelemetry/instrumentation-http`, this attribute is not yet
set when `parseSpanDescription` is called. The `sentry.kind` is derived from
`ReadableSpan.kind` later in the processing pipeline. This timing issue causes the span
`kind` to be `undefined` during description parsing, resulting in an incorrect `op`
(e.g., `'http'` instead of `'http.server'`) and preventing the correct inference of
transaction names from URL patterns.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Probably irrelevant for us, given otel spans go through our tracer and we set the attribute there 🤔
andreiborza
left a comment
andreiborza
left a comment
There was a problem hiding this comment.
LGTM, but could we wait with these kind of refactors until after the otel vendored instrumentations are removed from develop please? It's cumbersome to review otherwise.
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
nit: can/should we add a feat(core)!: to the PR/commit? It's behaviour breaking. Not sure how we handled non-API-breaking but still behaviour-breaking changes so far this.
| hook: 'preprocessSpan', | ||
| callback: (streamedSpanJSON: StreamedSpanJSON, hint?: { spanKind?: number }) => void, | ||
| ): () => void; | ||
| public on(hook: 'preprocessSpan', callback: (streamedSpanJSON: StreamedSpanJSON) => void): () => void; |
There was a problem hiding this comment.
that's fine IMHO, since nothing else is in the hint
| const opParts = ['http']; | ||
| const kind = attributes[SENTRY_KIND]; | ||
|
|
||
| switch (kind) { | ||
| case SpanKind.CLIENT: | ||
| case 'client': | ||
| opParts.push('client'); | ||
| break; | ||
| case SpanKind.SERVER: | ||
| case 'server': | ||
| opParts.push('server'); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Probably irrelevant for us, given otel spans go through our tracer and we set the attribute there 🤔
mydea
changed the title
kind for spans, move to sentry.kind attributekind for spans, move to sentry.kind attribute
3 participants
This removes
kindfrom start span options, and refactors all our usage to instead set thesentry.kindattribute directly.This is one of the things we rely on for span description inferral, so removing this takes us a step closer to this.
While at it, this also aligns the values to actually be lowercase as specced.