chore(telemetry): decouple telemetry writer from global config #10863
Conversation
|
|
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| def test_app_started_event_configuration_override( | ||
| test_agent_session, run_python_code_in_subprocess, tmpdir, env_var, value, expected_value | ||
| ): | ||
| # @pytest.mark.skip(reason="FIXME: This test needs to be updated.") |
There was a problem hiding this comment.
⚪ Code Quality Violation
| # @pytest.mark.skip(reason="FIXME: This test needs to be updated.") | |
| # @pytest.mark.skip(reason="FIXME(<owner>): This test needs to be updated.") |
comments must have ownership (...read more)
When using TODO or FIXME, specify who write the annotation. It's a best practice to remind you who created the annotation and have potential context and information about this issue.
| self._app_client_configuration_changed_event(cs) | ||
|
|
||
| def _telemetry_entry(self, cfg_name: str) -> Tuple[str, str, _ConfigSource]: | ||
| def _telemetry_entry(self, config: Any, cfg_name: str) -> Tuple[str, str, Any]: |
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
| test_agent_session, run_python_code_in_subprocess, tmpdir, env_var, value, expected_value | ||
| ): | ||
| # @pytest.mark.skip(reason="FIXME: This test needs to be updated.") | ||
| def test_app_started_event_configuration_override(test_agent_session, run_python_code_in_subprocess, tmpdir): |
There was a problem hiding this comment.
🔴 Code Quality Violation
function exceeds 200 lines (...read more)
This rule stipulates that functions in Python should not exceed 200 lines of code. The primary reason for this rule is to promote readability and maintainability of the code. When functions are concise and focused, they are easier to understand, test, and debug.
Long functions often indicate that a single function is doing too much. Adhering to the Single Responsibility Principle (SRP) can help avoid this. SRP states that a function should have only one reason to change. If a function is doing more than one thing, it can usually be split into several smaller, more specific functions.
In practice, to adhere to this rule, you can often break up long functions into smaller helper functions. If a piece of code within a function is independent and can be isolated, it is a good candidate to be moved into a separate function. This also increases code reusability. For instance, if a function process_data() is too long, you can identify independent tasks within it - such as clean_data(), transform_data(), and save_data() - and create separate functions for them. This makes the code easier to reason about and test, and promotes good coding practices.
|
That's great! Did we consider a callback/core event hub alternative approach to break the circular dependency? |
Currently, the telemetry writer relies on ddtrace.config to enable and send telemetry payloads, while also queuing telemetry configuration events. This creates a circular dependency that can be painful to manage.
With this change, ddtrace.config will no longer be used to initialize or send telemetry events. The telemerty_writer will access environment directly from
os.os.environ. The telemetry_writer can now queue configurations, metrics, and logs before the global config object is initialized,.Checklist
Reviewer Checklist