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

chore(profiling): add non-running tests for stack v2 #8576

Merged
merged 136 commits into from
Apr 1, 2024

Conversation

sanchda
Copy link
Contributor

@sanchda sanchda commented Mar 2, 2024

This PR creates a harness for independently building and running the stack v2 code. I did it this way for a few reasons

  • Don't want to add unnecessary overhead to our normal test process
  • Want to run the build for these native components under multiple static analysis tools
  • Want to run some of the tests for these components under sanitizers?
  • But also want to do exhaustive testing of the interfaces, since we've had a few regressions here

This PR includes both the tests and the fixups they demand. A separate PR will add a CI job to execute these (or will add it to a pre-existing job, TBD).

There IS some dead code in here, since we do provide the recipe for getting and running Infer. During testing, I found that I couldn't reliably get infer to find stdlib headers, which completely broke the quality of its output, so it won't actually be used. At the same time, I think we might want to use it someday, and the setup is a little fiddly, so I'm including it here.

I'm recommending a backport to 2.7 for this because the latest 2.7 releases make this feature available, but the implementation is subtly (and sometimes not-so-subtly) broken without the fixups here. Since this feature is so isolated (I'm the only one backporting anything related!), this feels safe...ish.

Checklist

  • Change(s) are motivated and described in the PR description
  • Testing strategy is described if automated tests are not included in the PR
  • Risks are described (performance impact, potential for breakage, maintainability)
  • Change is maintainable (easy to change, telemetry, documentation)
  • Library release note guidelines are followed or label changelog/no-changelog is set
  • Documentation is included (in-code, generated user docs, public corp docs)
  • Backport labels are set (if applicable)
  • If this PR changes the public interface, I've notified @DataDog/apm-tees.
  • If change 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.

Reviewer Checklist

  • Title is accurate
  • All changes are related to the pull request's stated goal
  • Description motivates each change
  • Avoids breaking API changes
  • Testing strategy adequately addresses listed risks
  • Change is maintainable (easy to change, telemetry, documentation)
  • Release note makes sense to a user of the library
  • Author has 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

sanchda and others added 30 commits February 21, 2024 15:55
Moved things around to streamline the build process a bit.  Also
discovered that recent changes were leaking intermediate artifacts into
the final .whl, which was causing auditwheel issues when the .so.debug
from libdatadog was being inspected.
@sanchda sanchda marked this pull request as ready for review March 22, 2024 16:32
@sanchda sanchda requested a review from a team as a code owner March 22, 2024 16:32
@sanchda sanchda requested a review from a team as a code owner March 22, 2024 17:05
@sanchda sanchda requested a review from a team as a code owner March 22, 2024 17:23
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Mar 22, 2024

Datadog Report

Branch report: sanchda/stackv2_initial_tests
Commit report: 13743c9
Test service: dd-trace-py

✅ 0 Failed, 171726 Passed, 1057 Skipped, 11h 28m 21.22s Total duration (39m 21.21s time saved)

@sanchda sanchda enabled auto-merge (squash) March 25, 2024 12:36
@sanchda sanchda merged commit eb635dd into main Apr 1, 2024
180 of 182 checks passed
@sanchda sanchda deleted the sanchda/stackv2_initial_tests branch April 1, 2024 18:35
github-actions bot pushed a commit that referenced this pull request Apr 1, 2024
This PR creates a harness for independently building and running the
stack v2 code. I did it this way for a few reasons

* Don't want to add unnecessary overhead to our normal test process
* Want to run the build for these native components under multiple
static analysis tools
* Want to run some of the tests for these components under sanitizers?
* But also want to do exhaustive testing of the interfaces, since we've
had a few regressions here

This PR includes both the tests and the fixups they demand. A separate
PR will add a CI job to execute these (or will add it to a pre-existing
job, TBD).

There IS some dead code in here, since we do provide the recipe for
getting and running Infer. During testing, I found that I couldn't
reliably get infer to find stdlib headers, which completely broke the
quality of its output, so it won't actually be used. At the same time, I
think we might want to use it someday, and the setup is a little fiddly,
so I'm including it here.

I'm recommending a backport to 2.7 for this because the latest 2.7
releases make this feature available, but the implementation is subtly
(and sometimes not-so-subtly) broken without the fixups here. Since this
feature is so isolated (I'm the only one backporting anything related!),
this feels safe...ish.

## 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] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [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] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.
- [x] If change 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`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has 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)

---------

Co-authored-by: sanchda <sanchda@users.noreply.github.com>
Co-authored-by: Gabriele N. Tornetta <P403n1x87@users.noreply.github.com>
(cherry picked from commit eb635dd)
sanchda added a commit that referenced this pull request Apr 2, 2024
…8824)

Backport eb635dd from #8576 to 2.7.

This PR creates a harness for independently building and running the
stack v2 code. I did it this way for a few reasons

* Don't want to add unnecessary overhead to our normal test process
* Want to run the build for these native components under multiple
static analysis tools
* Want to run some of the tests for these components under sanitizers?
* But also want to do exhaustive testing of the interfaces, since we've
had a few regressions here

This PR includes both the tests and the fixups they demand. A separate
PR will add a CI job to execute these (or will add it to a pre-existing
job, TBD).

There IS some dead code in here, since we do provide the recipe for
getting and running Infer. During testing, I found that I couldn't
reliably get infer to find stdlib headers, which completely broke the
quality of its output, so it won't actually be used. At the same time, I
think we might want to use it someday, and the setup is a little fiddly,
so I'm including it here.

I'm recommending a backport to 2.7 for this because the latest 2.7
releases make this feature available, but the implementation is subtly
(and sometimes not-so-subtly) broken without the fixups here. Since this
feature is so isolated (I'm the only one backporting anything related!),
this feels safe...ish.

## 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] Risks are described (performance impact, potential for breakage,
maintainability)
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] [Library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
are followed or label `changelog/no-changelog` is set
- [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] If this PR changes the public interface, I've notified
`@DataDog/apm-tees`.
- [x] If change 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`.

## Reviewer Checklist

- [x] Title is accurate
- [x] All changes are related to the pull request's stated goal
- [x] Description motivates each change
- [x] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- [x] Testing strategy adequately addresses listed risks
- [x] Change is maintainable (easy to change, telemetry, documentation)
- [x] Release note makes sense to a user of the library
- [x] Author has 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)

Co-authored-by: David Sanchez <838104+sanchda@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.7 changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants