feat(develop/span-first): Add implementation guidelines page #15717
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
sentrivana
left a comment
sentrivana
left a comment
There was a problem hiding this comment.
Thanks for writing this down. Looks great, see a few suggestions.
buenaflor
left a comment
buenaflor
left a comment
There was a problem hiding this comment.
nice! I added a suggestion regarding the release of Span-First PoC
lmk if that doesn't make sense in this page
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
|
|
||
| 1. Add the Span v2 Envelope (type), serialization logic and any utilities necessary to support sending a new envelope. See [Span Protocol](../span-protocol) for more details. | ||
| 2. Add the top-level `traceLifeCycle` (or `trace_life_cycle`) SDK init option which controls if traces should be sent as transactions or as spans (v2). | ||
| - The allowed values for this option MUST be `'static'` and `'strea'`. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Ivana Kellyer <ivana.kellyer@sentry.io> Co-authored-by: Giancarlo Buenaflor <giancarlo_buenaflor@yahoo.com>
Lms24
force-pushed
the
lms/feat-develop-span-first-guidelines
branch
from
9e9a6ee to
da22b11
Compare
|
|
||
| If you're implementing Span-First (as a PoC) in your SDK, take an iterative approach in which you implement the functionality incrementally. Here's a rough suggestion for iterations. | ||
|
|
||
| 1. Add the Span v2 Envelope (type), serialization logic and any utilities necessary to support sending a new envelope. See [Span Protocol](../span-protocol) for more details. |
There was a problem hiding this comment.
Bug: Incorrect relative links to span-protocol.mdx and span-api.mdx using ../ instead of ./.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
Relative links to span-protocol.mdx and span-api.mdx in implementation.mdx are incorrect. On lines 22, 29, 40, and 61, the paths use ../ which navigates up to the parent directory (/develop-docs/sdk/telemetry/) instead of referencing sibling files within the same spans/ directory. This prevents users from accessing essential documentation, fragmenting the learning experience.
💡 Suggested Fix
Change ../span-protocol to ./span-protocol (or span-protocol) and ../span-api to ./span-api (or span-api) on lines 22, 29, 40, and 61.
🤖 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: develop-docs/sdk/telemetry/spans/implementation.mdx#L22
Potential issue: Relative links to `span-protocol.mdx` and `span-api.mdx` in
`implementation.mdx` are incorrect. On lines 22, 29, 40, and 61, the paths use `../`
which navigates up to the parent directory (`/develop-docs/sdk/telemetry/`) instead of
referencing sibling files within the same `spans/` directory. This prevents users from
accessing essential documentation, fragmenting the learning experience.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6181154
| This document uses key words such as "MUST", "SHOULD", and "MAY" as defined in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt) to indicate requirement levels. | ||
| </Alert> | ||
|
|
||
| This document provides guidelines for implementing Span-First in SDKs. This is purposefully NOT a full specification. For exact specifications, refer to the other pages under [Spans](..). |
There was a problem hiding this comment.
Bug: Relative link [Spans](..) on line 16 points to the wrong parent directory.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
The relative link [Spans](..) on line 16 of implementation.mdx is incorrect. The (..) path points to the parent directory (develop-docs/sdk/telemetry/), which is the Telemetry overview page. The intended target is the Spans overview page at develop-docs/sdk/telemetry/spans/index.mdx. This causes the link to navigate to the wrong page or fail.
💡 Suggested Fix
Change [Spans](..) to [Spans](./) or [Spans](./index.mdx) to correctly reference the Spans overview page.
🤖 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: develop-docs/sdk/telemetry/spans/implementation.mdx#L16
Potential issue: The relative link `[Spans](..)` on line 16 of `implementation.mdx` is
incorrect. The `(..)` path points to the parent directory
(`develop-docs/sdk/telemetry/`), which is the Telemetry overview page. The intended
target is the Spans overview page at `develop-docs/sdk/telemetry/spans/index.mdx`. This
causes the link to navigate to the wrong page or fail.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6181154
| - TBD: Some SDKs already have `startSpan` or similar APIs. The migration path is still TBD but a decision can be made at a later stage. | ||
| 5. Implement the `captureSpan` [single-span processing pipeline](#single-span-processing-pipeline) | ||
| - Either reuse existing heuristics (e.g. flush when segment span ends) or build a simple span buffer to flush spans (e.g. similar to the existing buffers for logs or metrics). | ||
| - Implementing the more complex [Telemetry Buffer](./../../telemetry-buffer) can happen at a later stage. |
There was a problem hiding this comment.
Bug: Relative links to Telemetry Buffer on lines 37 and 113 point to the wrong directory.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
Relative links to Telemetry Buffer on lines 37 and 113 of implementation.mdx are incorrect. The paths ./../../telemetry-buffer and ../../telemetry-buffer both resolve to develop-docs/sdk/telemetry-buffer/. The correct path should be ../telemetry-buffer to navigate up one level to telemetry/ and then into telemetry-buffer/. This results in broken internal documentation links.
💡 Suggested Fix
Change [Telemetry Buffer](./../../telemetry-buffer) and [Telemetry Buffer](../../telemetry-buffer) to [Telemetry Buffer](../telemetry-buffer).
🤖 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: develop-docs/sdk/telemetry/spans/implementation.mdx#L37
Potential issue: Relative links to `Telemetry Buffer` on lines 37 and 113 of
`implementation.mdx` are incorrect. The paths `./../../telemetry-buffer` and
`../../telemetry-buffer` both resolve to `develop-docs/sdk/telemetry-buffer/`. The
correct path should be `../telemetry-buffer` to navigate up one level to `telemetry/`
and then into `telemetry-buffer/`. This results in broken internal documentation links.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 6181154
DESCRIBE YOUR PR
This PR adds a new "Implementation Guidelines" page to the Spans SDK develop section. Two purposes:
Anyone working on span first is encouraged to update this doc at any time!
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes:
LEGAL BOILERPLATE
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
EXTRA RESOURCES