chore(configs): make rc and http configs internal [3.0] [backport #12055 2.20] #12086
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prefixes the following `ddtrace.config` attributes with an `_`. These attributes are internal to the tracer and should not be referenced or modified by users: - http - http_server - trace_headers - header_is_traced - convert_rc_trace_sampling_rules - enable_remote_configuration - get_from Users should enable remote configuration and set `http` configurations via environment variables. - [ ] 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 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)
mabdinur
requested review from
rachelyangdog,
erikayasuda,
daniel-romano-DD,
taegyunkim and
quinna-h
|
|
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
Comment on lines
+1023
to
+1024
| else: | ||
| return None |
There was a problem hiding this comment.
⚪ Code Quality Violation
else is not necessary since the if clause has a return (...read more)
If the code in the if branch returns a value, do not have the else branch present.
| # If no data is submitted then the RC config has been deleted. Revert the settings. | ||
| base_rc_config = {n: None for n in self._config} | ||
|
|
||
| if config and "lib_config" in config: |
There was a problem hiding this comment.
🟠 Code Quality Violation
Found too many nested ifs within this condition (...read more)
Too many nested loops make the code hard to read and understand. Simplify your code by removing nesting levels and separate code in small units.
| def get_error_ranges(error_range_str): | ||
| # type: (str) -> List[Tuple[int, int]] | ||
| error_ranges = [] | ||
| error_range_str = error_range_str.strip() |
There was a problem hiding this comment.
🟠 Code Quality Violation
variable name is the same as a function parameter (...read more)
A function parameter should only be read and not be modified. If your intent is to modify the value of the parameter, return the value in the function and handle the new value in the caller of the function.
| break | ||
| telemetry_writer.add_configuration(self._name, self.value(), self.source()) | ||
|
|
||
| def set_value_source(self, value: Any, source: _ConfigSource) -> None: |
There was a problem hiding this comment.
🟠 Code Quality Violation
do not use Any, use a concrete type (...read more)
Use the Any type very carefully. Most of the time, the Any type is used because we do not know exactly what type is being used. If you want to specify that a value can be of any type, use object instead of Any.
Learn More
| Example of a converted rule: | ||
| '[{"sample_rate":1.0,"service":"my-service","resource":"*","name":"web.request","tags":{"care_about":"yes","region":"us-*"},provenance":"customer"}]' | ||
| """ | ||
| rc_rules = self._remove_invalid_rules(rc_rules) |
There was a problem hiding this comment.
🟠 Code Quality Violation
variable name is the same as a function parameter (...read more)
A function parameter should only be read and not be modified. If your intent is to modify the value of the parameter, return the value in the function and handle the new value in the caller of the function.
| """ | ||
| # DEV: Use `getattr()` to call our `__getattr__` helper | ||
| existing = getattr(self, integration) | ||
| settings = deepcopy(settings) |
There was a problem hiding this comment.
🟠 Code Quality Violation
variable name is the same as a function parameter (...read more)
A function parameter should only be read and not be modified. If your intent is to modify the value of the parameter, return the value in the function and handle the new value in the caller of the function.
mabdinur
changed the title
mabdinur
changed the title
Datadog ReportBranch report: ✅ 0 Failed, 130 Passed, 1468 Skipped, 4m 30.53s Total duration (36m 53.14s time saved) |
BenchmarksBenchmark execution time: 2025-01-24 22:34:48 Comparing candidate commit d278658 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 394 metrics, 2 unstable metrics. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backports: #12055
Checklist
Reviewer Checklist