Refactoring TraceHeader Calls

[atshaw43]

Issue #, if available:

Description of changes:
Problem 1
We have redundant code for creating a trace header.

Solution
Create a centralized function for creating trace headers from entities. This required NoOpSubsegment to have its traceId.

Problem 2
Why does NoOpSubsegment need a namespace and name? We have a bug where we are not correctly ending subsegments if they are NoOp. The subsegment gets created, but never ended by afterResponse because it is seen as a duplicate because its namespace and name is always empty.

Solution
Pass down the name and namespace in NoOpSubsegments.

https://github.com/aws/aws-xray-sdk-java/blob/master/aws-xray-recorder-sdk-aws-sdk/src/main/java/com/amazonaws/xray/handlers/TracingHandler.java

@Override
    public void afterResponse(Request<?> request, Response<?> response) {
        if (isSubsegmentDuplicate(recorder.getCurrentSubsegmentOptional(), request)) {
            Optional<Subsegment> currentSubsegmentOptional = recorder.getCurrentSubsegmentOptional();
            if (!currentSubsegmentOptional.isPresent()) {
                return;
            }
            Subsegment currentSubsegment = currentSubsegmentOptional.get();
            populateAndEndSubsegment(currentSubsegment, request, response, null);
        }
    }

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[codecov-commenter]

Codecov Report

Merging #361 (ce630b3) into master (fd93c8a) will increase coverage by 0.03%.
The diff coverage is 92.30%.

@@             Coverage Diff              @@
##             master     #361      +/-   ##
============================================
+ Coverage     58.35%   58.38%   +0.03%     
- Complexity     1226     1228       +2     
============================================
  Files           133      133              
  Lines          5016     5015       -1     
  Branches        595      590       -5     
============================================
+ Hits           2927     2928       +1     
- Misses         1815     1816       +1     
+ Partials        274      271       -3     

Impacted Files	Coverage Δ
.../main/java/com/amazonaws/xray/entities/Entity.java	100.00% <ø> (ø)
...main/java/com/amazonaws/xray/entities/Segment.java	100.00% <ø> (ø)
...va/com/amazonaws/xray/entities/NoOpSubSegment.java	85.43% <84.61%> (-0.14%)	⬇️
...aws/xray/proxies/apache/http/TracedHttpClient.java	40.47% <100.00%> (-2.06%)	⬇️
...mazonaws/xray/interceptors/TracingInterceptor.java	60.18% <100.00%> (+0.18%)	⬆️
...va/com/amazonaws/xray/handlers/TracingHandler.java	41.56% <100.00%> (-1.35%)	⬇️
.../amazonaws/xray/contexts/LambdaSegmentContext.java	79.16% <100.00%> (ø)
...n/java/com/amazonaws/xray/entities/Subsegment.java	50.00% <100.00%> (+10.00%)	⬆️
...va/com/amazonaws/xray/entities/SubsegmentImpl.java	73.52% <100.00%> (+1.88%)	⬆️
.../java/com/amazonaws/xray/entities/TraceHeader.java	90.58% <100.00%> (+0.46%)	⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[srprash]

This code doesn't look equivalent to the one it's replacing.
In the left side code, the parent id as the TraceHeader argument is either currentSubsegment's id or null. Only the isSampled is fetched from either previousSubsegment or the recorder.getCurrentSegment().
In the right side code, the whole TraceHeader will be created either from previousSubsegment or currentSubsegment.

[atshaw43]

Wow, good catch! This was a bug.
I looked at the original code and I don't think we even need to use the previous subsegment here.

[srprash]

If this NoOpSubsegment is never emitted and never propagated, can we set a static id here? We should avoid unnecessary computation if we can.

[atshaw43]

Removed.
I added it when I noticed parentId was missing in NoOpSubsegments in trace headers. Then I saw we don't actually add it to the header when it is not sampled. Forgot to remove it after that.

[srprash]

Do we need to necessarily override the isSampled() method from Entity?
Is it possible for Entity types and subtypes to directly use this method from Entity without an override?

[atshaw43]

Good point, removed.