docs(python): Update stream mode in tracing files (tracing part 3/3)#18533
docs(python): Update stream mode in tracing files (tracing part 3/3)#18533inventarSarah wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
a1d549e to
9da9987
Compare
| ) as span: | ||
| try: | ||
| # Check inventory availability | ||
| inventory_start = time.time() |
There was a problem hiding this comment.
Bug: Code examples in the span-metrics documentation use functions from the time, psutil, and os modules without importing them, which will cause a NameError at runtime.
Severity: HIGH
Suggested Fix
Add import time to the E-Commerce examples. Add import psutil and import os to the Data Processing Pipeline examples to make the code snippets self-contained and runnable.
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: docs/platforms/python/tracing/span-metrics/examples.mdx#L675
Potential issue: In the E-Commerce "Order processing service" examples for both
Transaction and Stream modes, the code uses `time.time()` to measure execution time but
fails to import the `time` module. Similarly, the Data Processing Pipeline examples use
`psutil.cpu_percent()`, `psutil.Process().memory_info()`, and `os.path.getsize()`
without importing the `psutil` or `os` modules. Users copying these examples will
encounter `NameError` exceptions at runtime when these functions are called.
Also affects:
docs/platforms/python/tracing/span-metrics/examples.mdx:614docs/platforms/python/tracing/span-metrics/examples.mdx:801docs/platforms/python/tracing/span-metrics/examples.mdx:809docs/platforms/python/tracing/span-metrics/examples.mdx:887docs/platforms/python/tracing/span-metrics/examples.mdx:895
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
added the missing imports
| # (which resets the propagation context) and before start_span | ||
| # (which is when sampling happens), in order to be available | ||
| # in the traces_sampler | ||
| sentry_sdk.Scope.set_custom_sampling_context({"hasRecentErrors": True}) |
There was a problem hiding this comment.
Bug: The code calls sentry_sdk.Scope.set_custom_sampling_context(...) directly on the class. If this is an instance method, it will raise a TypeError at runtime.
Severity: HIGH
Suggested Fix
Verify if set_custom_sampling_context is intended to be a class method. If it is, ensure it is decorated with @classmethod in the SDK. If it is an instance method, the documentation should be updated to show how to get a scope instance and call the method on that instance, for example: with sentry_sdk.push_scope() as scope: scope.set_custom_sampling_context(...).
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: docs/platforms/python/tracing/configure-sampling/index.mdx#L216
Potential issue: The documentation provides code examples that call
`sentry_sdk.Scope.set_custom_sampling_context(...)` directly on the class. This pattern
is only valid if the method is a `@classmethod` or `@staticmethod`. If it is a standard
instance method, which is the default for Python classes, this call will fail at runtime
with a `TypeError` because no instance of the class (`self`) is provided. The method is
not documented in the public API, so its intended usage is unclear, but the provided
examples are likely to be incorrect and will crash for users who copy them.
Also affects:
docs/platforms/python/tracing/configure-sampling/index.mdx:313docs/platforms/python/tracing/configure-sampling/index.mdx:434docs/platforms/python/tracing/configure-sampling/index.mdx:539
There was a problem hiding this comment.
@alexander-alderman-webb @ericapisani
Can you please take a look at this (also flagged in the migration guide) -> I took this directly from the SKILLS file (in case this requires a fix, we need to update that file as well)
| sentry_sdk.traces.continue_trace(headers) | ||
|
|
||
| `continue_trace()` returns a transaction, but does not start it. To start the transaction, use `start_transaction()`. | ||
| with sentry_sdk.traces.start_span(name="handle request"): | ||
| ... |
There was a problem hiding this comment.
Bug: Code examples use the sentry_sdk.traces submodule, which appears to be an undocumented and potentially non-existent public API, likely causing an AttributeError for users.
Severity: MEDIUM
Suggested Fix
Verify that sentry_sdk.traces is a correct and public API in the Python SDK. If it is, add the necessary import statements and documentation for the submodule. If it is not, update the code examples to use the correct, publicly documented API, such as sentry_sdk.start_span().
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:
platform-includes/distributed-tracing/custom-instrumentation/python.mdx#L45-L48
Potential issue: The pull request introduces multiple new code examples for Python
tracing that utilize functions from a `sentry_sdk.traces` submodule, such as
`sentry_sdk.traces.start_span()`. However, this submodule is not documented anywhere in
the existing repository, and the standard, documented API for similar functionality is
`sentry_sdk.start_span()`. If the `sentry_sdk.traces` submodule is not a valid, publicly
exposed API in the Sentry Python SDK, users who copy these examples will encounter an
`AttributeError` at runtime, preventing them from successfully implementing the
documented feature.
Also affects:
docs/platforms/python/tracing/configure-sampling/index.mdx:217~221docs/platforms/python/tracing/span-metrics/examples.mdx:100~103
DESCRIBE YOUR PR
This PR updates pages under Tracing with information about the new stream mode and the new Span APIs.
Part 3 out of 3
(result of splitting up PR #18511 into smaller parts)
Important
Broken links: I added links to the New Spans guide on these pages, but since this guide does not exist in this branch, we get a linting error.
Should only be merged after #18456
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