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.

Sign up for GitHub

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

Jump to bottom

refactor(debugging): make safety module internal #6810

Merged
merged 5 commits into from
Sep 13, 2023
Merged

refactor(debugging): make safety module internal #6810

merged 5 commits into from
Sep 13, 2023

Conversation

P403n1x87
Copy link
Contributor

@P403n1x87 P403n1x87 commented Aug 31, 2023 •
edited by emmettbutler
Loading

The safety submodule of ddtrace.debugging has been made public (according to the naming conventions) by accident. The API of this module is not documented and we do not expect any usage outside the library itself.

Checklist

  • Change(s) are motivated and described in the PR description.
  • Testing strategy is described if automated tests are not included in the PR.
  • Risk is outlined (performance impact, potential for breakage, maintainability, etc).
  • Change is maintainable (easy to change, telemetry, documentation).
  • Library release note guidelines are followed. If no release note is required, add label changelog/no-changelog.
  • Documentation is included (in-code, generated user docs, public corp docs).
  • Backport labels are set (if applicable)

Reviewer Checklist

  • Title is accurate.
  • No unnecessary changes are introduced.
  • Description motivates each change.
  • Avoids breaking API changes unless absolutely necessary.
  • Testing strategy adequately addresses listed risk(s).
  • Change is maintainable (easy to change, telemetry, documentation).
  • Release note makes sense to a user of the library.
  • Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment.
  • Backport labels are set in a manner that is consistent with the release branch maintenance policy
  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

@P403n1x87 P403n1x87 added changelog/no-changelog A changelog entry is not required for this PR. Dynamic Instrumentation Dynamic Instrumentation/Live Debugger labels Aug 31, 2023
@P403n1x87 P403n1x87 added this to the v2.0.0 milestone Aug 31, 2023
@P403n1x87 P403n1x87 requested a review from a team as a code owner August 31, 2023 15:42
shatzi
shatzi previously approved these changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@shatzi shatzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

@majorgreys majorgreys requested review from a team as code owners September 7, 2023 20:39
@majorgreys majorgreys requested review from Kyle-Verhoog, duncanista, majorgreys and emmettbutler and removed request for a team September 7, 2023 20:39
@majorgreys majorgreys force-pushed the 2.x branch 2 times, most recently from 146b009 to 0b1dab1 Compare September 7, 2023 20:47
Yun-Kim and others added 2 commits September 8, 2023 09:30
@Yun-Kim @majorgreys
feat: drop support for Python < 3.7
a4b338e 
Includes:
- pinning images and package dependencies in CircleCI
- removing stale `master` branch
- dropping pylons framework test
- updating versioning documentation
- drop Python < 3.7 as supported on setup.py
- removing pylons/boto from CI suitespec
- release note for 2.0
@Yun-Kim @majorgreys
chore: remove deprecated items (DataDog#6580)
b9c9efd 
This PR adds a note to upgrade to 2.x in `upgrading.rst` and removes all
deprecated items slated for removal in 2.0.0. This includes:

- `DD_GEVENT_PATCH_ALL`: no special configuration is now necessary to
make `ddtrace-run` work with gevent.
- `DD_AWS_TAG_ALL_PARAMS`: the boto/botocore/aiobotocore integrations no
longer collect all API parameters by default.
- `DD_REMOTECONFIG_POLL_SECONDS`: replaced by
`DD_REMOTE_CONFIG_POLL_INTERVAL_SECONDS`
- ASM deprecated constants including ``APPSEC_ENABLED``,
``APPSEC_JSON``, ``APPSEC_EVENT_RULE_VERSION``,
``APPSEC_EVENT_RULE_ERRORS``,
``APPSEC_EVENT_RULE_LOADED``, ``APPSEC_EVENT_RULE_ERROR_COUNT``,
``APPSEC_WAF_DURATION``, ``APPSEC_WAF_DURATION_EXT``,
``APPSEC_WAF_TIMEOUTS``, ``APPSEC_WAF_VERSION``,
``APPSEC_ORIGIN_VALUE``, ``APPSEC_BLOCKED``,
``IAST_JSON``, ``IAST_ENABLED``, ``IAST_CONTEXT_KEY``. These constants
were meant for private use only and should not affect existing code.
- ``ddtrace.contrib.grpc.constants.GRPC_PORT_KEY``: replaced by
`ddtrace.ext.net.TARGET_PORT`
- ``ddtrace.ext.cassandra.ROW_COUNT``, ``ddtrace.ext.mongo.ROW_COUNT``,
``ddtrace.ext.sql.ROW_COUNT``: replaced by `ddtrace.ext.db.ROWCOUNT`
- `ddtrace.filters.TraceCiVisibilityFilter`: removed as this was for
private use only and does not affect existing code.
- `ddtrace.contrib.starlette.get_resource` and
`ddtrace.contrib.starlette.span_modifier` and
`ddtrace.contrib.fastapi.span_modifier`: the fastapi and starlette
integrations now provide the full route and not just mounted route for
sub-applications by default.
- `ddtrace.contrib.starlette.config['aggregate_resources']` and
`ddtrace.contrib.fastapi.config['aggregate_resources']`: the starlette
and fastapi integrations no longer have the option to aggregate
resources as this occurs by default now.
- `DD_TRACE_OBFUSCATION_QUERY_STRING_PATTERN`: replaced by
`DD_TRACE_OBFUSCATION_QUERY_STRING_REGEXP`.

Additionally, the `pep562` dependency and references to it have been
removed as it is no longer needed after dropping support for Python <
3.7.

Note that `DD_CALL_BASIC_CONFIG` and `DD_LOG_FORMAT` are removed in

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
@P403n1x87 P403n1x87 dismissed shatzi’s stale review September 8, 2023 14:20

The merge-base changed after approval.

@Yun-Kim @P403n1x87
Drop support for Python < 3.7
d91463d 
Includes:
- pinning images and package dependencies in CircleCI
- removing stale `master` branch
- dropping pylons framework test
- updating versioning documentation
- drop Python < 3.7 as supported on setup.py
- removing pylons/boto from CI suitespec
- release note for 2.0
@P403n1x87 P403n1x87 force-pushed the refactor/debugger-internalise-safety branch from bc1014f to a600c9b Compare September 8, 2023 15:33
@P403n1x87
refactor(debugging): make safety module internal
2ab2b73 
The safety submodule of `ddtrace.debugging` has been made public
(according to the naming conventions) by accident. The API of this
module is not documented and we do not expect any usage outside
the library itself.
@P403n1x87 P403n1x87 force-pushed the refactor/debugger-internalise-safety branch from a600c9b to 2ab2b73 Compare September 8, 2023 15:35
emmettbutler
emmettbutler previously approved these changes Sep 8, 2023
View reviewed changes
@P403n1x87 P403n1x87 requested a review from shatzi September 11, 2023 13:42
@majorgreys majorgreys requested a review from a team as a code owner September 12, 2023 15:41
@majorgreys majorgreys requested a review from a team as a code owner September 12, 2023 15:41
@majorgreys majorgreys requested review from tabgok and removed request for a team September 12, 2023 15:41
@P403n1x87 P403n1x87 dismissed emmettbutler’s stale review September 12, 2023 15:41

The merge-base changed after approval.

@emmettbutler emmettbutler self-requested a review September 13, 2023 15:43
@emmettbutler emmettbutler enabled auto-merge (squash) September 13, 2023 15:43
@emmettbutler
Copy link
Collaborator

@P403n1x87 the flask-poc system test has failed a few times in a row on this change. do you think it's related?

@emmettbutler emmettbutler merged commit e14eb7e into DataDog:2.x Sep 13, 2023
@P403n1x87
Copy link
Contributor Author

@P403n1x87 the flask-poc system test has failed a few times in a row on this change. do you think it's related?

I don't think so 🤔

@P403n1x87 P403n1x87 deleted the refactor/debugger-internalise-safety branch September 13, 2023 18:57
majorgreys pushed a commit that referenced this pull request Sep 15, 2023
@P403n1x87 @Yun-Kim @majorgreys
refactor(debugging): make safety module internal (#6810)
8110ba2 
The safety submodule of `ddtrace.debugging` has been made public
(according to the naming conventions) by accident. The API of this
module is not documented and we do not expect any usage outside the
library itself.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
majorgreys pushed a commit that referenced this pull request Sep 18, 2023
@P403n1x87 @Yun-Kim @majorgreys
refactor(debugging): make safety module internal (#6810)
914bbbb 
The safety submodule of `ddtrace.debugging` has been made public
(according to the naming conventions) by accident. The API of this
module is not documented and we do not expect any usage outside the
library itself.

## Checklist

- [x] Change(s) are motivated and described in the PR description.
- [x] Testing strategy is described if automated tests are not included
in the PR.
- [x] Risk is outlined (performance impact, potential for breakage,
maintainability, etc).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed. If no release note is required, add label
`changelog/no-changelog`.
- [x] Documentation is included (in-code, generated user docs, [public
corp docs](https://github.com/DataDog/documentation/)).
- [x] Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist

- [x] Title is accurate.
- [x] No unnecessary changes are introduced.
- [x] Description motivates each change.
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes unless absolutely necessary.
- [x] Testing strategy adequately addresses listed risk(s).
- [x] Change is maintainable (easy to change, telemetry, documentation).
- [x] Release note makes sense to a user of the library.
- [x] Reviewer has explicitly acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment.
- [x] Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
- [x] If this PR touches code that signs or publishes builds or
packages, or handles credentials of any kind, I've requested a review
from `@DataDog/security-design-and-guidance`.
- [x] This PR doesn't touch any of that.

---------

Co-authored-by: Yun Kim <[email protected]>
Co-authored-by: Yun Kim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shatzi shatzi shatzi approved these changes

@emmettbutler emmettbutler emmettbutler approved these changes

@Kyle-Verhoog Kyle-Verhoog Awaiting requested review from Kyle-Verhoog Kyle-Verhoog was automatically assigned from DataDog/python-guild

@duncanista duncanista Awaiting requested review from duncanista duncanista was automatically assigned from DataDog/apm-python

@majorgreys majorgreys Awaiting requested review from majorgreys majorgreys was automatically assigned from DataDog/apm-core-python

@tabgok tabgok Awaiting requested review from tabgok tabgok was automatically assigned from DataDog/apm-framework-integrations-reviewers-py

Assignees
No one assigned
Labels
changelog/no-changelog A changelog entry is not required for this PR. Dynamic Instrumentation Dynamic Instrumentation/Live Debugger
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@P403n1x87 @emmettbutler @shatzi @Yun-Kim