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

chore(service.naming): schematize django db calls #6335

Merged
merged 6 commits into from
Jul 13, 2023
Merged

chore(service.naming): schematize django db calls #6335

merged 6 commits into from
Jul 13, 2023

Conversation

tabgok
Copy link
Contributor

@tabgok tabgok commented Jul 11, 2023 •
edited by ZStriker19
Loading

Django has a separate mechanism for determining service name when using an underlying database. This change adds the schematized service.naming to those Django DB calls.

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

@tabgok tabgok added the changelog/no-changelog A changelog entry is not required for this PR. label Jul 11, 2023
@tabgok tabgok self-assigned this Jul 11, 2023
@tabgok tabgok marked this pull request as ready for review July 12, 2023 13:48
@tabgok tabgok requested review from a team as code owners July 12, 2023 13:48
@tabgok
chore(service.naming) - schematize django db calls
00df715 
Django has a separate mechanism for determining service name when using
an underlying database.  This change adds the schematized service.naming
to those Django DB calls.
@tabgok tabgok force-pushed the teague.bick/service.naming/fix-django branch from d9c7e82 to 00df715 Compare July 12, 2023 13:49
@pr-commenter
Copy link

pr-commenter bot commented Jul 12, 2023 •
edited
Loading

Benchmarks

Comparing candidate commit ac736b9 in PR branch teague.bick/service.naming/fix-django with baseline commit 11a8d09 in branch 1.x.

Found 1 performance improvements and 1 performance regressions! Performance is the same for 92 cases.

scenario:flasksimple-appsec-get

  • 🟥 execution_time [+0.283ms; +0.314ms] or [+4.725%; +5.238%]

scenario:flasksimple-appsec-telemetry

  • 🟩 execution_time [-0.344ms; -0.314ms] or [-5.444%; -4.976%]

@tabgok tabgok enabled auto-merge (squash) July 12, 2023 15:38
@tabgok tabgok changed the title chore(service.naming) - schematize django db calls chore(service.naming): schematize django db calls Jul 12, 2023
@tabgok tabgok merged commit 2dbe9cf into 1.x Jul 13, 2023
@tabgok tabgok deleted the teague.bick/service.naming/fix-django branch July 13, 2023 11:12
@github-actions github-actions bot added this to the v1.17.0 milestone Jul 13, 2023
@github-actions github-actions bot mentioned this pull request Jul 13, 2023
Merged
16 tasks
github-actions bot pushed a commit that referenced this pull request Jul 13, 2023
@tabgok @github-actions
chore(service.naming) - schematize django db calls (#6335)
aa2e761 
Django has a separate mechanism for determining service name when using
an underlying database. This change adds the schematized service.naming
to those Django DB calls.

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

- [ ] Title is accurate.
- [ ] No unnecessary changes are introduced.
- [ ] Description motivates each change.
- [ ] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

(cherry picked from commit 2dbe9cf)
ZStriker19 pushed a commit that referenced this pull request Jul 13, 2023
@github-actions @tabgok
chore(service.naming): schematize django db calls [backport 1.16] (#6358
6d2c89a 
)

Backport 2dbe9cf from #6335 to 1.16.

Django has a separate mechanism for determining service name when using
an underlying database. This change adds the schematized service.naming
to those Django DB calls.

## 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)

Co-authored-by: Teague Bick <teague.bick@datadoghq.com>
romainkomorndatadog pushed a commit that referenced this pull request Aug 8, 2023
@tabgok @romainkomorndatadog
chore(service.naming) - schematize django db calls (#6335)
30cc087 
Django has a separate mechanism for determining service name when using
an underlying database. This change adds the schematized service.naming
to those Django DB calls.

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

- [ ] Title is accurate.
- [ ] No unnecessary changes are introduced.
- [ ] Description motivates each change.
- [ ] Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ZStriker19 ZStriker19 ZStriker19 approved these changes

@amarziali amarziali amarziali approved these changes

@Yun-Kim Yun-Kim Awaiting requested review from Yun-Kim Yun-Kim is a code owner automatically assigned from DataDog/apm-core-python

@jbertran jbertran Awaiting requested review from jbertran jbertran is a code owner automatically assigned from DataDog/apm-framework-integrations-reviewers-py

Assignees

@tabgok tabgok

Labels
changelog/no-changelog A changelog entry is not required for this PR.
Projects
None yet
Milestone
v1.17.0
Development

Successfully merging this pull request may close these issues.
5 participants
@tabgok @amarziali @ZStriker19 @emmettbutler @brettlangdon