dd-trace-py
dd-trace-py copied to clipboard
refactor: extract reusable logic from Agent Writer to HTTP Writer
This is preparing ground for CI Visibility Agentless support that will reuse retry logic from Agent Writer. Also we would like to keep the possibility for metrics calculation and submission in background.
https://github.com/DataDog/dd-trace-py/pull/3654#discussion_r866849329
Checklist
- [ ] Added to the correct milestone.
- [ ] Tests provided or description of manual testing performed is included in the code or PR.
- [ ] Library documentation is updated.
- [ ] Corp site documentation is updated (link to the PR).
Codecov Report
Merging #3690 (38a8d8c) into 1.x (27aceb6) will decrease coverage by
0.11%. The diff coverage is52.76%.
@@ Coverage Diff @@
## 1.x #3690 +/- ##
==========================================
- Coverage 77.55% 77.44% -0.12%
==========================================
Files 666 671 +5
Lines 51334 51566 +232
==========================================
+ Hits 39811 39934 +123
- Misses 11523 11632 +109
| Impacted Files | Coverage Δ | |
|---|---|---|
| ddtrace/contrib/pytest/constants.py | 0.00% <0.00%> (ø) |
|
| ddtrace/contrib/pytest_bdd/__init__.py | 0.00% <0.00%> (ø) |
|
| ddtrace/contrib/pytest_bdd/constants.py | 0.00% <0.00%> (ø) |
|
| ddtrace/ext/test.py | 0.00% <0.00%> (ø) |
|
| ddtrace/profiling/exporter/pprof_pb2.py | 0.00% <0.00%> (ø) |
|
| ddtrace/profiling/exporter/pprof_pre319_pb2.py | 0.00% <0.00%> (ø) |
|
| tests/contrib/grpc/hello_pb2.py | 69.56% <50.00%> (-30.44%) |
:arrow_down: |
| ddtrace/contrib/pytest_bdd/plugin.py | 53.33% <53.33%> (ø) |
|
| ddtrace/contrib/pytest/plugin.py | 72.54% <75.00%> (-0.48%) |
:arrow_down: |
| ddtrace/internal/writer.py | 84.08% <77.27%> (+0.17%) |
:arrow_up: |
| ... and 2 more |
:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more
@Kyle-Verhoog I'm working on a fresh attempt at this change here, and I'm not sure I can see the need for the "has-a" relationship you described here. I think I've got a change that pretty well separates the concerns that are AgentWriter-specific from those that apply to the HTTPWriter using an "is-a" relationship. I'm not asking you to review that change, but could you provide any more detail on what "transmission state and corresponding metrics" would be better encapsulated in an instance attribute than a subclass relationship? Thanks a lot for loading this old comment back into your brain.
@emmettbutler I think where I was coming from is the leaking of concerns between the base and subclasses. By doing an "is-a" relationship it is very easy to move a method up a layer of the classes and have the concerns leaked to the base.
I think "is-a" can work it's just much easier to fall into implicitly coupling the classes because it's so easy to share state between them.