-
Notifications
You must be signed in to change notification settings - Fork 581
ref(types): add partial type hints for SamplingContext
#5442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| "headers": list[tuple[bytes, bytes]], | ||
| "state": dict[str, Any], | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_ASGIScope marks optional ASGI fields as required
Medium Severity
The _ASGIScope TypedDict uses total=True (the default), marking all fields as required. Per the ASGI HTTP connection scope spec, server, client, scheme, raw_path, and state are optional. Developers relying on these type hints might access fields like scope["client"] or scope["server"] directly and hit KeyError at runtime when the ASGI server omits them. These fields need NotRequired wrapping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cursoragent As per official ASGI Spec, I have made fields optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unable to authenticate your request. Please make sure to connect your GitHub account to Cursor. Go to Cursor
| "rq_job": NotRequired[Any], | ||
| "aws_event": NotRequired[Any], | ||
| "aws_context": NotRequired[Any], | ||
| "gcp_env": NotRequired[dict[str, Any]], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing gcp_event field in _SamplingContextTyped TypedDict
Low Severity
The _SamplingContextTyped TypedDict includes gcp_env but is missing the gcp_event field. The GCP integration passes both "gcp_env" and "gcp_event" in the custom sampling context. This omission means developers using GCP Cloud Functions won't see gcp_event in autocomplete, despite it being available at runtime. The Union with dict[str, Any] prevents a type error, but the field is still undocumented in the typed definition.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cursoragent I have added gcp_event as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unable to authenticate your request. Please make sure to connect your GitHub account to Cursor. Go to Cursor
5e310dc to
b1d3de3
Compare
|
Looks like I need write permission in the main repo for the tests to pass. Someone please let me know, what I can do meanwhile. |


Description
Adds type hints for
SamplingContextto improve developer experience when implementingtraces_samplercallback. Previously,SamplingContextwas typed asDict[str, Any], developers had no way to discover available fields without reading source code or inspecting field in runtime.Motivation
While implementing custom transaction sampling logic to control which transactions are sent from the client, I noticed there were no type hints for the
sampling_contextparameter intraces_samplercallback. This made it difficult to know what fields were available without inspecting thesample_contextin runtime usingipdb. This deteriorated developer experience.Before
After
Issues
None for this PR - this is an enhancement for developer experience
Notes
Union[SamplingContextTyped, dict[str, Any]]was added as type toSamplingContextto not break SDK specific internals, as well as allow developers to access integration-specific fieldsSamplingContextTypedto maintain type safety.Example Usage
Reminders
tox -e linterspassedif TYPE_CHECKING:block (no runtime overhead)reffor refactor)pip install -e .