Splitting out the jaxrs2 filters #273
Conversation
Signed-off-by: Debosmit Ray <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #273 +/- ##
===========================================
+ Coverage 82.33% 82.5% +0.17%
- Complexity 535 542 +7
===========================================
Files 87 89 +2
Lines 2021 2041 +20
Branches 237 237
===========================================
+ Hits 1664 1684 +20
Misses 259 259
Partials 98 98
Continue to review full report at Codecov.
|
| @@ -36,30 +33,22 @@ | |||
| private final Tracer tracer; | |||
| private final TraceContext traceContext; | |||
|
|
|||
| private final ClientSpanCreationFilter spanCreationFilter; | |||
| private final ClientSpanInjectionFilter spaninjectionFilter; | |||
There was a problem hiding this comment.
s/spaninjectionFilter/spanInjectionFilter
| public ClientFilter(Tracer tracer, TraceContext traceContext) { | ||
| this.tracer = tracer; | ||
| this.traceContext = traceContext; | ||
|
|
||
| this.spanCreationFilter = new ClientSpanCreationFilter(tracer, traceContext); | ||
| this.spaninjectionFilter = new ClientSpanInjectionFilter(tracer, traceContext); | ||
| } | ||
|
|
||
| @Override | ||
| public void filter(ClientRequestContext clientRequestContext) throws IOException { |
There was a problem hiding this comment.
Just so I follow, for your use case, you'll extend the ClientFilter and then override this filter function to add your baggage between the spanCreationFilter and spanInjectionFilter?
What do we do in the future when other people want to add extra functionality? Will they add their functionality into your filter?
There was a problem hiding this comment.
then someone can add a CompositeFilter
There was a problem hiding this comment.
right, so is that the plan?
There was a problem hiding this comment.
there's no plan - it's not a problem today, and it's solvable in the future with composite filter, so we don't need to over-engineer today for non-existing need
|
every commit has to be signed off, you can squash your commits under one with the sign off |
Signed-off-by: Debosmit Ray <[email protected]>
ghost
assigned 
| @Provider | ||
| @ConstrainedTo(RuntimeType.CLIENT) | ||
| @Slf4j | ||
| public class ClientSpanCreationFilter implements ClientRequestFilter { |
There was a problem hiding this comment.
I don't think that we should be releasing OSS without proper Javadocs.
Signed-off-by: Debosmit Ray [email protected]