jaeger
jaeger copied to clipboard
Visualize uninstrumented services in the dependency diagrams
Which problem is this PR solving?
- Closes #3804
- Replaces #4853
Description of the changes
- Updated
GetDependencies
to identify and include client spans without corresponding server spans, ensuring a complete dependency graph. - Implemented
updateServiceDependencyLinks
to streamline the update of service dependencies, reducing code duplication. - Enabled the representation of uninstrumented services in the dependency graph by inferring names from client spans lacking server spans.
How was this change tested?
- Introduced TestInferredServiceDependency to specifically evaluate the functionality of inferred service dependencies in the scenario of uninstrumented services.
- Created a client span without a corresponding server span and asserted the correct formation of dependencies, ensuring the inclusion of inferred services in the dependency graph.
- Employed assertions to confirm the presence and accuracy of inferred service dependencies, ensuring they are adequately represented with the correct attributes and relationships.
Checklist
- [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
- for
jaeger
:make lint test
- for
jaeger-ui
:yarn lint
andyarn test
- for
Ref PR: https://github.com/jaegertracing/jaeger/pull/4853
+1, we are awaiting this feature. Currently the deep dependency graph is not showing the relationship with the databases and other non-instrumented services, for the given service.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 93.76%. Comparing base (
a72dfc3
) to head (d031261
).
Additional details and impacted files
@@ Coverage Diff @@
## main #5062 +/- ##
==========================================
- Coverage 95.20% 93.76% -1.44%
==========================================
Files 343 343
Lines 16813 16846 +33
==========================================
- Hits 16006 15795 -211
- Misses 608 869 +261
+ Partials 199 182 -17
Flag | Coverage Δ | |
---|---|---|
badger | 10.43% <0.00%> (-0.04%) |
:arrow_down: |
cassandra-3.x | 18.31% <0.00%> (-0.07%) |
:arrow_down: |
cassandra-4.x | 18.31% <0.00%> (-0.07%) |
:arrow_down: |
elasticsearch-5.x | 20.81% <0.00%> (-0.10%) |
:arrow_down: |
elasticsearch-6.x | 20.81% <0.00%> (-0.13%) |
:arrow_down: |
elasticsearch-7.x | 20.87% <0.00%> (-0.12%) |
:arrow_down: |
elasticsearch-8.x | 21.10% <0.00%> (-0.03%) |
:arrow_down: |
grpc | ? |
|
kafka | 10.13% <0.00%> (-0.04%) |
:arrow_down: |
opensearch-1.x | 20.90% <0.00%> (-0.07%) |
:arrow_down: |
opensearch-2.x | 20.93% <0.00%> (-0.05%) |
:arrow_down: |
unittests | 91.74% <100.00%> (+0.01%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
This is a dupe of #4853 which, as you can see, was a very long conversation. This PR is a 10 line change. It's very frustrating for the maintainers who need to spend a ton of time fixing such a small issue. I spoke to the author as well and explained this decision we are making. The decision is that this will not be merged as it would need to be rebased and once again create a lot of work for the team. Sorry.