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

Deprecate undocumented propagate_traces option #3106

Closed
antonpirker opened this issue May 27, 2024 · 6 comments · Fixed by #3899
Closed

Deprecate undocumented propagate_traces option #3106

antonpirker opened this issue May 27, 2024 · 6 comments · Fixed by #3899
Labels
Feature: Performance Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest!

Comments

@antonpirker
Copy link
Member

antonpirker commented May 27, 2024

The option is not documented and it is True by default. We should deprecated it and remove in the next major.

This option was later superseded by trace_propagation_targets.

@szokeasaurusrex szokeasaurusrex added Triaged Has been looked at recently during old issue triage Good first issue Good for newcomers labels Oct 7, 2024
@antonpirker antonpirker added the Hacktoberfest 🎃 Issues eligible for Hacktoberfest! label Oct 10, 2024
nellaG added a commit to nellaG/sentry-python that referenced this issue Nov 2, 2024
`propagate_traces` is not documented and superseded by
`trace_propagation_targets`.

Refs getsentry#3106
nellaG added a commit to nellaG/sentry-python that referenced this issue Nov 2, 2024
`propagate_traces` is not documented and superseded by
`trace_propagation_targets`.

Refs getsentry#3106
@antonpirker antonpirker added the Breaking change needs to go out in a major label Dec 19, 2024
@mgaligniana
Copy link
Contributor

mgaligniana commented Dec 23, 2024

Hi @antonpirker. I see there is already a PR to delete the flag, but should we start triggering a DeprecationWarning? Could I create a pull request for that? I haven't found how to proceed with a deprecation on the Sentry contribution guidelines. Once I did for Django on this way. Thank you!

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 23, 2024
@szokeasaurusrex
Copy link
Member

@mgaligniana we usually just use DeprecationWarning directly, along with a message stating that the option is deprecated. We also need to make sure we remove any usages of the deprecated item from the SDK codebase, to avoid triggering the warning ourselves.

@mgaligniana
Copy link
Contributor

@mgaligniana we usually just use DeprecationWarning directly, along with a message stating that the option is deprecated. We also need to make sure we remove any usages of the deprecated item from the SDK codebase, to avoid triggering the warning ourselves.

Thank you @szokeasaurusrex! So if I understood well the only thing missing here is add the warning in the current pull request.

I was just trying to contribute! If there is anything I can contribute, let me know! Thanks and have a great new year! 🌟

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Dec 30, 2024
@szokeasaurusrex
Copy link
Member

Thank you, we really appreciate it!

So if I understood well the only thing missing here is add the warning in the current pull request.

That is incorrect. For this issue, we need a new pull request to add the deprecation warning. The existing PR removes the propagate_traces option completely. We will need to wait to merge the existing PR until we are releasing a new major version, but we can add the deprecation warning now.

Happy new year to you, as well!

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Jan 2, 2025
mgaligniana added a commit to mgaligniana/sentry-python that referenced this issue Jan 3, 2025
mgaligniana added a commit to mgaligniana/sentry-python that referenced this issue Jan 3, 2025
mgaligniana added a commit to mgaligniana/sentry-python that referenced this issue Jan 3, 2025
@mgaligniana
Copy link
Contributor

Happy friday @szokeasaurusrex !

I just created the pull request adding the deprecation warning!

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 3, 2025
mgaligniana added a commit to mgaligniana/sentry-python that referenced this issue Jan 3, 2025
@szokeasaurusrex
Copy link
Member

Thanks @mgaligniana, really appreciate the contribution! We will try to take a look at it soon

@getsantry getsantry bot removed the status in GitHub Issues with 👀 3 Jan 3, 2025
antonpirker added a commit to mgaligniana/sentry-python that referenced this issue Jan 14, 2025
antonpirker added a commit to mgaligniana/sentry-python that referenced this issue Jan 14, 2025
@antonpirker antonpirker removed Triaged Has been looked at recently during old issue triage Breaking change needs to go out in a major labels Jan 14, 2025
antonpirker added a commit that referenced this issue Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature: Performance Good first issue Good for newcomers Hacktoberfest 🎃 Issues eligible for Hacktoberfest!
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants