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

fix(ci_visibility): use default tracer in CI Visibility (#9328) #9350

Merged
merged 4 commits into from
Jun 13, 2024

Conversation

romainkomorndatadog
Copy link
Collaborator

@romainkomorndatadog romainkomorndatadog commented May 22, 2024

This reverts commit 2008dd7 and reinstates the changes from #9016 .

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.

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

@romainkomorndatadog romainkomorndatadog self-assigned this May 22, 2024
@romainkomorndatadog romainkomorndatadog requested review from a team as code owners May 22, 2024 16:06
@romainkomorndatadog romainkomorndatadog changed the title chore(ci_visibility): revert revert default tracer change (#9328) fix(ci_visibility): use default tracer in CI Visibility (#9328) May 22, 2024
@romainkomorndatadog
Copy link
Collaborator Author

After chatting with @brettlangdon , we're leaning towards not backporting this to 2.9.

I suppose if #9349 merges and backports into 2.9 first, then this change would be safe enough to backport as well.

@datadog-dd-trace-py-rkomorn
Copy link

Datadog Report

Branch report: romain.komorn/CIVIS-9440/revert_revert
Commit report: 2cb5100
Test service: dd-trace-py

✅ 0 Failed, 174675 Passed, 1104 Skipped, 11h 18m 2.7s Total duration (20m 18.32s time saved)

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 7.69231% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 10.28%. Comparing base (66b96e9) to head (2cb5100).
Report is 2 commits behind head on main.

Files Patch % Lines
tests/contrib/pytest/test_pytest_snapshot.py 0.00% 11 Missing ⚠️
ddtrace/internal/ci_visibility/recorder.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #9350       +/-   ##
===========================================
- Coverage   76.02%   10.28%   -65.75%     
===========================================
  Files        1294     1264       -30     
  Lines      122816   120958     -1858     
===========================================
- Hits        93370    12439    -80931     
- Misses      29446   108519    +79073     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@emmettbutler emmettbutler left a comment

Choose a reason for hiding this comment

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

Does dogweb CI pass when run against this commit?

@romainkomorndatadog
Copy link
Collaborator Author

@emmettbutler

Does dogweb CI pass when run against this commit?

Not yet, it needs #9349 first.

Copy link
Collaborator

@emmettbutler emmettbutler left a comment

Choose a reason for hiding this comment

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

I'm going to hold off on approving this until there's a clear indication that it's been tested against dogweb CI.

@romainkomorndatadog
Copy link
Collaborator Author

@emmettbutler this never broke dogweb CI, dogweb CI broke because of the bug fixed in #9349 and dogweb's use of tracer.configure().

Bluntly put, dogweb CI isn't this PR's problem and it shouldn't be blocking this fix. We shouldn't be blocking a release because of this.

@emmettbutler emmettbutler self-requested a review June 11, 2024 17:45
Copy link
Collaborator

@emmettbutler emmettbutler left a comment

Choose a reason for hiding this comment

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

@romainkomorndatadog thanks for clarifying!

@emmettbutler emmettbutler enabled auto-merge (squash) June 13, 2024 14:52
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Jun 13, 2024

Datadog Report

Branch report: romain.komorn/CIVIS-9440/revert_revert
Commit report: 63b7178
Test service: dd-trace-py

✅ 0 Failed, 118195 Passed, 59404 Skipped, 4h 16m 34.18s Total duration (7m 21.71s time saved)

@pr-commenter
Copy link

pr-commenter bot commented Jun 13, 2024

Benchmarks

Benchmark execution time: 2024-06-13 16:50:03

Comparing candidate commit 63b7178 in PR branch romain.komorn/CIVIS-9440/revert_revert with baseline commit 128b80d in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 221 metrics, 9 unstable metrics.

@emmettbutler emmettbutler merged commit 99c74e6 into main Jun 13, 2024
202 of 209 checks passed
@emmettbutler emmettbutler deleted the romain.komorn/CIVIS-9440/revert_revert branch June 13, 2024 16:54
github-actions bot pushed a commit that referenced this pull request Jun 13, 2024
This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
(cherry picked from commit 99c74e6)
github-actions bot pushed a commit that referenced this pull request Jun 13, 2024
This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
(cherry picked from commit 99c74e6)
github-actions bot pushed a commit that referenced this pull request Jun 13, 2024
This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
(cherry picked from commit 99c74e6)
github-actions bot pushed a commit that referenced this pull request Jun 13, 2024
This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Emmett Butler <723615+emmettbutler@users.noreply.github.com>
(cherry picked from commit 99c74e6)
@romainkomorndatadog
Copy link
Collaborator Author

Well, crap. This needed to merge after #9349 .

gnufede pushed a commit that referenced this pull request Jun 21, 2024
…port 2.10] (#9544)

Backport 99c74e6 from #9350 to 2.10.

This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com>
romainkomorndatadog added a commit that referenced this pull request Jun 24, 2024
…port 2.8] (#9542)

Backport 99c74e6 from #9350 to 2.8.

This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com>
Co-authored-by: Federico Mon <federico.mon@datadoghq.com>
romainkomorndatadog added a commit that referenced this pull request Jun 26, 2024
…port 2.7] (#9541)

Backport 99c74e6 from #9350 to 2.7.

This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com>
romainkomorndatadog added a commit that referenced this pull request Jul 1, 2024
…port 2.9] (#9543)

Backport 99c74e6 from #9350 to 2.9.

This reverts commit 2008dd7 and
reinstates the changes from #9016 .

## 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`.

## 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: Romain Komorn <136473744+romainkomorndatadog@users.noreply.github.com>
Co-authored-by: Federico Mon <federico.mon@datadoghq.com>
Co-authored-by: Brett Langdon <brett.langdon@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants