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

feat(botocore): add override service name to botocore integration #9856

Merged
merged 23 commits into from
Jul 22, 2024

Conversation

quinna-h
Copy link
Contributor

@quinna-h quinna-h commented Jul 16, 2024

Motivation:

Currently traces in botocore derive their service name from the service and endpoint, ex. aws.s3 or aws.sqs, and there is no way to change the service name. Other integrations allow for changing the default service name either by setting environment variables, or configuring with config['service'] or config['service_name']. This PR adds support for these methods of overriding the service name, which allows individual traces to have custom service names.

Examples:

@mock_s3
ddtrace.config.botocore['service_name'] = "botocore" # this sets service name to botocore.s3
ddtrace.config.botocore['service'] = "boto-service" # this overrides previous and sets service name to boto-service.s3

export DD_BOTOCORE_SERVICE="boto" # this overrides previous settings and sets service & service name to boto.s3

Checklist

  • PR author has checked that all the criteria below are met
  • The PR description includes an overview of the change
  • The PR description articulates the motivation for the change
  • The change includes tests OR the PR description describes a testing strategy
  • The PR description notes risks associated with the change, if any
  • Newly-added code is easy to change
  • The change follows the library release note guidelines
  • The change includes or references documentation updates if necessary
  • Backport labels are set (if applicable)

Reviewer Checklist

  • Reviewer has checked that all the criteria below are met
  • Title is accurate
  • All changes are related to the pull request's stated goal
  • Avoids breaking API changes
  • Testing strategy adequately addresses listed risks
  • Newly-added code is easy to change
  • Release note makes sense to a user of the library
  • If necessary, 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

Copy link
Contributor

github-actions bot commented Jul 16, 2024

CODEOWNERS have been resolved as:

releasenotes/notes/add-c0f1d24cf11f23b7.yaml                            @DataDog/apm-python
ddtrace/contrib/botocore/patch.py                                       @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/botocore/services/bedrock.py                            @DataDog/ml-observability
ddtrace/contrib/botocore/services/kinesis.py                            @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/botocore/services/sqs.py                                @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/botocore/services/stepfunctions.py                      @DataDog/apm-core-python @DataDog/apm-idm-python
docs/configuration.rst                                                  @DataDog/apm-core-python
tests/contrib/botocore/test.py                                          @DataDog/apm-core-python @DataDog/apm-idm-python

@pr-commenter
Copy link

pr-commenter bot commented Jul 17, 2024

Benchmarks

Benchmark execution time: 2024-07-18 21:24:11

Comparing candidate commit 11b474c in PR branch quinna/AIDM-210/botocore-service-naming with baseline commit ac1308b in branch main.

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

scenario:sethttpmeta-useragentvariant_exists_2

  • 🟥 max_rss_usage [+1.435MB; +1.494MB] or [+7.032%; +7.321%]

scenario:sethttpmeta-useragentvariant_exists_3

  • 🟥 max_rss_usage [+1.461MB; +1.525MB] or [+7.167%; +7.483%]

@datadog-datadog-prod-us1
Copy link
Contributor

datadog-datadog-prod-us1 bot commented Jul 18, 2024

Library Vulnerabilities

Critical badge  High badge  Medium badge

@quinna-h quinna-h changed the title Add override service name to botocore integration fix(botocore): adds override service name to botocore integration Jul 18, 2024
@quinna-h quinna-h changed the title fix(botocore): adds override service name to botocore integration feat(botocore): adds override service name to botocore integration Jul 18, 2024
@datadog-dd-trace-py-rkomorn
Copy link

datadog-dd-trace-py-rkomorn bot commented Jul 18, 2024

Datadog Report

Branch report: quinna/AIDM-210/botocore-service-naming
Commit report: 97f31b5
Test service: dd-trace-py

✅ 0 Failed, 180 Passed, 783 Skipped, 7m 18.92s Total duration (23m 2.29s time saved)

docs/configuration.rst Outdated Show resolved Hide resolved
…ocore-service-naming' into quinna/AIDM-210/botocore-service-naming
@quinna-h quinna-h marked this pull request as ready for review July 19, 2024 15:16
@quinna-h quinna-h requested review from a team as code owners July 19, 2024 15:16
Copy link
Contributor

@erikayasuda erikayasuda left a comment

Choose a reason for hiding this comment

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

LGTM but I will leave the official approval to IDM and MLObs since these are their files

docs/configuration.rst Show resolved Hide resolved
releasenotes/notes/add-c0f1d24cf11f23b7.yaml Outdated Show resolved Hide resolved
@quinna-h quinna-h enabled auto-merge (squash) July 19, 2024 17:50
@quinna-h quinna-h disabled auto-merge July 19, 2024 18:12
@quinna-h quinna-h enabled auto-merge (squash) July 19, 2024 18:14
@erikayasuda
Copy link
Contributor

@DataDog/ml-observability Could we get a review on this PR? 🙏 The automation didn't automatically tag someone from the team for some reason.

@quinna-h quinna-h changed the title feat(botocore): adds override service name to botocore integration feat(botocore): add override service name to botocore integration Jul 22, 2024
@quinna-h quinna-h merged commit 2bb0c4b into main Jul 22, 2024
81 checks passed
@quinna-h quinna-h deleted the quinna/AIDM-210/botocore-service-naming branch July 22, 2024 17:05
github-actions bot pushed a commit that referenced this pull request Jul 22, 2024
…9856)

Motivation:

Currently traces in botocore derive their service name from the service
and endpoint, ex. `aws.s3` or `aws.sqs`, and there is no way to change
the service name. Other integrations allow for changing the default
service name either by setting environment variables, or configuring
with `config['service']` or `config['service_name']`. This PR adds
support for these methods of overriding the service name, which allows
individual traces to have custom service names.

Examples:

```
@mock_s3
ddtrace.config.botocore['service_name'] = "botocore" # this sets service name to botocore.s3
ddtrace.config.botocore['service'] = "boto-service" # this overrides previous and sets service name to boto-service.s3
```

`export DD_BOTOCORE_SERVICE="boto" # this overrides previous settings
and sets service & service name to boto.s3
`
## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

---------

Co-authored-by: Zachary Groves <[email protected]>
Co-authored-by: erikayasuda <[email protected]>
(cherry picked from commit 2bb0c4b)
quinna-h added a commit that referenced this pull request Jul 23, 2024
…ckport 2.10] (#9894)

Backport 2bb0c4b from #9856 to 2.10.

Motivation:

Currently traces in botocore derive their service name from the service
and endpoint, ex. `aws.s3` or `aws.sqs`, and there is no way to change
the service name. Other integrations allow for changing the default
service name either by setting environment variables, or configuring
with `config['service']` or `config['service_name']`. This PR adds
support for these methods of overriding the service name, which allows
individual traces to have custom service names.

Examples:

```
@mock_s3
ddtrace.config.botocore['service_name'] = "botocore" # this sets service name to botocore.s3
ddtrace.config.botocore['service'] = "boto-service" # this overrides previous and sets service name to boto-service.s3
```

`export DD_BOTOCORE_SERVICE="boto" # this overrides previous settings
and sets service & service name to boto.s3
`
## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, 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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

Co-authored-by: Quinna Halim <[email protected]>
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.

5 participants