Skip to content
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

unclear what args can/should be provided to start_transaction or what they mean #5082

Closed
admackin opened this issue May 27, 2022 · 2 comments · Fixed by #9850
Closed

unclear what args can/should be provided to start_transaction or what they mean #5082

admackin opened this issue May 27, 2022 · 2 comments · Fixed by #9850
Assignees
Labels
performance Platform: Python Team: Web Backend SDKs team-web-sdk-backend Type: Content Issues about the contents of our docs

Comments

@admackin
Copy link

Core or SDK?

Platform/SDK

Which part? Which one?

Python SDK

Description

I want to manually instrument using start_transaction function as documented here. The docs mention the function but I can't find info anywhere on that docs page or in the pydocs in the source code what kwargs I can or should pass, and what their effect is. what does op mean? what about name. Similar issues apply to start_span

Suggested Solution

document kwargs we might want to pass to start_transaction and start_span and what they actually do.

@getsentry-release
Copy link
Contributor

Routing to @getsentry/team-web-sdk-backend for triage. ⏲️

@stephanie-anderson stephanie-anderson added the Type: Content Issues about the contents of our docs label Mar 4, 2024
@szokeasaurusrex szokeasaurusrex self-assigned this Mar 5, 2024
antonpirker added a commit to getsentry/sentry-python that referenced this issue Mar 11, 2024
This PR adds be type hints for the `**kwargs` that can be passed to `sentry_sdk.start_transaction`, thereby clearly documenting the parameters that can be passed directly in the code.

Ref getsentry/sentry-docs#5082

  - We intend to add to the docs page at least the most useful arguments defined in the `TransactionKwargs` type that this PR introduces.

---------

Co-authored-by: Anton Pirker <anton.pirker@sentry.io>
szokeasaurusrex added a commit to getsentry/sentry-python that referenced this issue Mar 28, 2024
This has been deprecated for 4 years, so I suppose we can remove it now in 2.0.

Removing the __new__ method also fixes our API docs for the Span and Transaction constructors. 

Partially addresses getsentry/sentry-docs#5082
szokeasaurusrex added a commit to getsentry/sentry-python that referenced this issue Mar 28, 2024
This change moves the Transaction constructor's parameter doctsring from the __init__ method to the class's docstring. This way, the API docs display the parameter descriptions under the class. When the docstring is defined on __init__, the parameter descriptions are missing from the API docs.

This change also documents the kwargs parameter in the API docs.

ref getsentry/sentry-docs#5082
szokeasaurusrex added a commit to getsentry/sentry-python that referenced this issue Mar 28, 2024
szokeasaurusrex added a commit to getsentry/sentry-python that referenced this issue Mar 28, 2024
szokeasaurusrex added a commit to getsentry/sentry-python that referenced this issue Mar 28, 2024
Repeating the doc comments also on the kwargs typed dicts enables better hinting in VSCode

ref: getsentry/sentry-docs#5082
szokeasaurusrex added a commit to getsentry/sentry-python that referenced this issue Mar 28, 2024
This commit adds documentation for all start_transaction parameters. It also removes the StartTransactionKwargs typeddict, instead placing the custom_sampling_context parameter directly on the method and deleting the client parameter, which appears not to be used.

ref getsentry/sentry-docs#5082
szokeasaurusrex added a commit to getsentry/sentry-python that referenced this issue Mar 28, 2024
Although the @scopemethod decorator should take care of adding this documentation comment to start_transaction, the decorator does not appear to work when hovering over the sentry_sdk.start_transaction function in VSCode. Adding the doc comment explicitly allows the documentation to be shown when hovering in VSCode, but it does not change the output of our API docs, which still uses the @scopemethod output.

This is the final PR in this repository that addresses getsentry/sentry-docs#5082; the last step to closing that issue will be to add a link in the docs to start_transaction in our API docs. We can only do this once 2.0 is released.
@szokeasaurusrex
Copy link
Member

Version 2.0 of the Sentry SDK will contain better documentation for start_transaction in the form of type hints and documentation of all parameters in the API docs. The last step to resolving this issue will be to add a link to the API docs to direct users to discover the available parameters there. We can only do this once 2.0 is released

@szokeasaurusrex szokeasaurusrex removed their assignment Apr 19, 2024
@szokeasaurusrex szokeasaurusrex self-assigned this Apr 29, 2024
szokeasaurusrex added a commit that referenced this issue Apr 29, 2024
Link to the `start_transaction` API docs, which describe all the possible arguments to `start_transaction`.

Closes GH-5082
szokeasaurusrex added a commit that referenced this issue May 28, 2024
Link to the `start_transaction` API docs, which describe all the possible arguments to `start_transaction`.

Closes GH-5082
szokeasaurusrex added a commit that referenced this issue May 28, 2024
Link to the `start_transaction` API docs, which describe all the possible arguments to `start_transaction`.

Closes GH-5082
matejminar pushed a commit that referenced this issue Jun 6, 2024
Link to the `start_transaction` API docs, which describe all the possible arguments to `start_transaction`.

Closes GH-5082
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Platform: Python Team: Web Backend SDKs team-web-sdk-backend Type: Content Issues about the contents of our docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants