From 8058202f780189fed18e772837851aac5fd73173 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Fri, 12 Aug 2022 14:17:48 -0700 Subject: [PATCH 1/9] Revert "Don't lock segment from subsegment (#306)" This reverts commit d5ccae28236b274fccb7f44b027b8c8e819b0f99. --- .../amazonaws/xray/entities/EntityImpl.java | 25 ++++++++++++------- .../amazonaws/xray/entities/SegmentImpl.java | 9 ++++--- .../strategy/DefaultStreamingStrategy.java | 2 +- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java index 27f6b10d..0725b648 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java @@ -36,7 +36,6 @@ import com.fasterxml.jackson.databind.ser.std.ToStringSerializer; import com.google.errorprone.annotations.concurrent.GuardedBy; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -198,7 +197,7 @@ protected EntityImpl(AWSXRayRecorder creator, String name) { this.creator = creator; this.name = name; - this.subsegments = Collections.synchronizedList(new ArrayList<>()); + this.subsegments = new ArrayList<>(); this.subsegmentsLock = new ReentrantLock(); this.cause = new Cause(); this.http = new HashMap<>(); @@ -532,21 +531,25 @@ public void setParentId(@Nullable String parentId) { @Override public List getSubsegments() { - return subsegments; + synchronized (lock) { + return subsegments; + } } @JsonIgnore @Override public List getSubsegmentsCopy() { - return new ArrayList<>(subsegments); + synchronized (lock) { + return new ArrayList<>(subsegments); + } } @Override public void addSubsegment(Subsegment subsegment) { synchronized (lock) { checkAlreadyEmitted(); + subsegments.add(subsegment); } - subsegments.add(subsegment); } @Override @@ -672,8 +675,8 @@ public void incrementReferenceCount() { synchronized (lock) { checkAlreadyEmitted(); referenceCount++; + totalSize.increment(); } - totalSize.increment(); } @Override @@ -703,7 +706,9 @@ public int getReferenceCount() { */ @Override public LongAdder getTotalSize() { - return totalSize; + synchronized (lock) { + return totalSize; + } } /** @@ -765,8 +770,10 @@ public String prettySerialize() { @Override public void removeSubsegment(Subsegment subsegment) { - subsegments.remove(subsegment); - getParentSegment().getTotalSize().decrement(); + synchronized (lock) { + subsegments.remove(subsegment); + getParentSegment().getTotalSize().decrement(); + } } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java index 7014fdb7..fc4b05aa 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java @@ -34,7 +34,8 @@ public class SegmentImpl extends EntityImpl implements Segment { protected Map service; @JsonIgnore - private volatile boolean sampled; + @GuardedBy("lock") + private boolean sampled; @SuppressWarnings({ "unused", "nullness" }) private SegmentImpl() { @@ -87,15 +88,17 @@ public boolean isRecording() { @Override public boolean isSampled() { - return sampled; + synchronized (lock) { + return sampled; + } } @Override public void setSampled(boolean sampled) { synchronized (lock) { checkAlreadyEmitted(); + this.sampled = sampled; } - this.sampled = sampled; } @Override diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java index bc3d57fe..89487815 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java @@ -67,7 +67,7 @@ public int getMaxSegmentSize() { */ @Override public boolean requiresStreaming(Segment segment) { - if (segment.isSampled()) { + if (segment.isSampled() && null != segment.getTotalSize()) { return segment.getTotalSize().intValue() > maxSegmentSize; } return false; From 45f041bb9dab7810db8bb0b955009f0ffa57ec6d Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:13:01 -0700 Subject: [PATCH 2/9] Revert "Keep track of emitted in emitter. (#279)" This reverts commit d66d69a0eb5ab1d1dae2fc1b6a14608ac3781b13. --- .../amazonaws/xray/emitters/UDPEmitter.java | 36 ++++++++----------- .../amazonaws/xray/entities/DummySegment.java | 5 --- .../xray/entities/DummySubsegment.java | 5 --- .../com/amazonaws/xray/entities/Entity.java | 10 ------ .../amazonaws/xray/entities/EntityImpl.java | 19 ++-------- .../amazonaws/xray/entities/NoOpSegment.java | 5 --- .../xray/entities/NoOpSubSegment.java | 5 --- .../amazonaws/xray/entities/SegmentImpl.java | 2 +- .../xray/entities/SubsegmentImpl.java | 2 +- .../strategy/DefaultStreamingStrategy.java | 1 + .../amazonaws/xray/AWSXRayRecorderTest.java | 2 -- 11 files changed, 19 insertions(+), 73 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/emitters/UDPEmitter.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/emitters/UDPEmitter.java index 4e9244a1..60be7029 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/emitters/UDPEmitter.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/emitters/UDPEmitter.java @@ -83,24 +83,21 @@ public boolean sendSegment(Segment segment) { if (logger.isDebugEnabled()) { logger.debug(segment.prettySerialize()); } - if (segment.compareAndSetEmitted(false, true)) { - byte[] bytes = (PROTOCOL_HEADER + PROTOCOL_DELIMITER + segment.serialize()).getBytes(StandardCharsets.UTF_8); + + byte[] bytes = (PROTOCOL_HEADER + PROTOCOL_DELIMITER + segment.serialize()).getBytes(StandardCharsets.UTF_8); - if (bytes.length > UDP_PACKET_LIMIT) { - List subsegments = segment.getSubsegmentsCopy(); - logger.debug("Segment too large, sending subsegments to daemon first. bytes " + bytes.length + " subsegemnts " - + subsegments.size()); - for (Subsegment subsegment : subsegments) { - sendSubsegment(subsegment); - segment.removeSubsegment(subsegment); - } - bytes = (PROTOCOL_HEADER + PROTOCOL_DELIMITER + segment.serialize()).getBytes(StandardCharsets.UTF_8); - logger.debug("New segment size. bytes " + bytes.length); + if (bytes.length > UDP_PACKET_LIMIT) { + List subsegments = segment.getSubsegmentsCopy(); + logger.debug("Segment too large, sending subsegments to daemon first. bytes " + bytes.length + " subsegemnts " + + subsegments.size()); + for (Subsegment subsegment : subsegments) { + sendSubsegment(subsegment); + segment.removeSubsegment(subsegment); } - return sendData(bytes, segment); - } else { - return false; + bytes = (PROTOCOL_HEADER + PROTOCOL_DELIMITER + segment.serialize()).getBytes(StandardCharsets.UTF_8); + logger.debug("New segment size. bytes " + bytes.length); } + return sendData(bytes, segment); } /** @@ -113,13 +110,8 @@ public boolean sendSubsegment(Subsegment subsegment) { if (logger.isDebugEnabled()) { logger.debug(subsegment.prettyStreamSerialize()); } - if (subsegment.compareAndSetEmitted(false, true)) { - return sendData((PROTOCOL_HEADER + PROTOCOL_DELIMITER + - subsegment.streamSerialize()).getBytes(StandardCharsets.UTF_8), - subsegment); - } else { - return false; - } + return sendData((PROTOCOL_HEADER + PROTOCOL_DELIMITER + subsegment.streamSerialize()).getBytes(StandardCharsets.UTF_8), + subsegment); } private boolean sendData(byte[] data, Entity entity) { diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java index d1c0961e..7db7f7e8 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java @@ -307,11 +307,6 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } - @Override - public boolean compareAndSetEmitted(boolean current, boolean next) { - return false; - } - @Override public String serialize() { return ""; diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java index 16bf12ef..eaa37b61 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java @@ -289,11 +289,6 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } - @Override - public boolean compareAndSetEmitted(boolean current, boolean next) { - return false; - } - @Override public String serialize() { return ""; diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java index 00e5c4b8..fbf8dff4 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java @@ -600,18 +600,8 @@ default void run(Runnable runnable, AWSXRayRecorder recorder) { boolean isEmitted(); - /** - * @deprecated Use {@link #compareAndSetEmitted(boolean, boolean)} - */ - @Deprecated void setEmitted(boolean emitted); - /** - * Checks whether this {@link Entity} currently has emitted state of {@code current} and if so, set emitted state to - * {@code next}. Returns {@code true} if the state was updated, or {@code false} otherwise. - */ - boolean compareAndSetEmitted(boolean current, boolean next); - String serialize(); String prettySerialize(); diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java index 0725b648..6e143847 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java @@ -144,10 +144,6 @@ public abstract class EntityImpl implements Entity { @GuardedBy("lock") private boolean emitted = false; - @JsonIgnore - @GuardedBy("lock") - protected boolean ended = false; - static { /* * Inject the CauseSerializer and StackTraceElementSerializer classes into the local mapper such that they will serialize @@ -213,7 +209,7 @@ protected EntityImpl(AWSXRayRecorder creator, String name) { } /** - * Checks if the entity has already been ended. + * Checks if the entity has already been emitted to the X-Ray daemon. * * @throws AlreadyEmittedException * if the entity has already been emitted to the X-Ray daemon and the ContextMissingStrategy of the @@ -222,7 +218,7 @@ protected EntityImpl(AWSXRayRecorder creator, String name) { */ protected void checkAlreadyEmitted() { synchronized (lock) { - if (ended || emitted) { + if (emitted) { getCreator().getContextMissingStrategy().contextMissing("Segment " + getName() + " has already been emitted.", AlreadyEmittedException.class); } @@ -733,17 +729,6 @@ public void setEmitted(boolean emitted) { } } - @Override - public boolean compareAndSetEmitted(boolean current, boolean next) { - synchronized (lock) { - if (emitted == current) { - emitted = next; - return true; - } - return false; - } - } - @Override public String serialize() { synchronized (lock) { diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java index e406e9ac..8dd6baa2 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java @@ -372,11 +372,6 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } - @Override - public boolean compareAndSetEmitted(boolean current, boolean next) { - return false; - } - @Override public String serialize() { return ""; diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java index ca82b064..1f10fd80 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java @@ -321,11 +321,6 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } - @Override - public boolean compareAndSetEmitted(boolean current, boolean next) { - return false; - } - @Override public String serialize() { return ""; diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java index fc4b05aa..6a23b461 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java @@ -75,7 +75,7 @@ public boolean end() { boolean shouldEmit = referenceCount <= 0; if (shouldEmit) { checkAlreadyEmitted(); - ended = true; + setEmitted(true); } return shouldEmit; } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java index 657b3747..b8a2cb9d 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java @@ -69,7 +69,7 @@ public boolean end() { boolean shouldEmit = parentSegment.decrementReferenceCount() && parentSegment.isSampled(); if (shouldEmit) { checkAlreadyEmitted(); - ended = true; + setEmitted(true); } return shouldEmit; } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java index 89487815..8ec734c5 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java @@ -107,6 +107,7 @@ private boolean stream(Entity entity, Emitter emitter) { //Stream the subtrees that are ready. for (Subsegment child : streamable) { emitter.sendSubsegment(child); + child.setEmitted(true); entity.removeSubsegment(child); } diff --git a/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java b/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java index c292fbd3..84598322 100644 --- a/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java +++ b/aws-xray-recorder-sdk-core/src/test/java/com/amazonaws/xray/AWSXRayRecorderTest.java @@ -750,7 +750,6 @@ public void testNoOpSegment() throws Exception { assertThat(segment.getReferenceCount()).isZero(); segment.removeSubsegment(Subsegment.noOp(AWSXRay.getGlobalRecorder(), true)); segment.setEmitted(true); - segment.compareAndSetEmitted(false, true); assertThat(segment.isEmitted()).isFalse(); assertThat(segment.serialize()).isEmpty(); assertThat(segment.prettySerialize()).isEmpty(); @@ -868,7 +867,6 @@ public void testNoOpSubsegment() throws Exception { assertThat(subsegment.getReferenceCount()).isZero(); subsegment.removeSubsegment(Subsegment.noOp(AWSXRay.getGlobalRecorder(), true)); subsegment.setEmitted(true); - subsegment.compareAndSetEmitted(false, true); assertThat(subsegment.isEmitted()).isFalse(); assertThat(subsegment.serialize()).isEmpty(); assertThat(subsegment.prettySerialize()).isEmpty(); From 36f94ea854d5bcd039542529623032ebb6018a69 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:14:07 -0700 Subject: [PATCH 3/9] Revert "Remove subsegments lock (#278)" This reverts commit adfb395adbf084e71f8fb78bfd11c60b2d410931. --- .../amazonaws/xray/entities/DummySegment.java | 5 --- .../xray/entities/DummySubsegment.java | 5 --- .../com/amazonaws/xray/entities/Entity.java | 11 +------ .../amazonaws/xray/entities/EntityImpl.java | 31 ++++++++++++------- .../amazonaws/xray/entities/NoOpSegment.java | 5 --- .../xray/entities/NoOpSubSegment.java | 5 --- .../strategy/DefaultStreamingStrategy.java | 23 ++++++++++---- 7 files changed, 37 insertions(+), 48 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java index 7db7f7e8..9c8afc85 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java @@ -241,11 +241,6 @@ public List getSubsegments() { return list; } - @Override - public List getSubsegmentsCopy() { - return new ArrayList<>(list); - } - @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java index eaa37b61..ee29f2f5 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java @@ -223,11 +223,6 @@ public List getSubsegments() { return list; } - @Override - public List getSubsegmentsCopy() { - return new ArrayList<>(list); - } - @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java index fbf8dff4..3cbc81ed 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java @@ -158,7 +158,7 @@ default void run(Runnable runnable, AWSXRayRecorder recorder) { void setNamespace(String namespace); /** - * @return an unused {@link ReentrantLock} + * @return the subsegmentsLock * * @deprecated This is for internal use of the SDK and will be made private. */ @@ -367,18 +367,9 @@ default void run(Runnable runnable, AWSXRayRecorder recorder) { /** * @return the subsegments - * - * @deprecated Use {@link #getSubsegmentsCopy()}. */ - @Deprecated List getSubsegments(); - /** - * Returns a copy of the currently added subsegments. Updates to the returned {@link List} will not be reflected in the - * {@link Entity}. - */ - List getSubsegmentsCopy(); - /** * Adds a subsegment. * diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java index 6e143847..aecc41a0 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java @@ -532,19 +532,16 @@ public List getSubsegments() { } } - @JsonIgnore - @Override - public List getSubsegmentsCopy() { - synchronized (lock) { - return new ArrayList<>(subsegments); - } - } - @Override public void addSubsegment(Subsegment subsegment) { synchronized (lock) { checkAlreadyEmitted(); - subsegments.add(subsegment); + getSubsegmentsLock().lock(); + try { + subsegments.add(subsegment); + } finally { + getSubsegmentsLock().unlock(); + } } } @@ -553,8 +550,13 @@ public void addException(Throwable exception) { synchronized (lock) { checkAlreadyEmitted(); setFault(true); - cause.addExceptions(creator.getThrowableSerializationStrategy() - .describeInContext(this, exception, subsegments)); + getSubsegmentsLock().lock(); + try { + cause.addExceptions(creator.getThrowableSerializationStrategy() + .describeInContext(this, exception, subsegments)); + } finally { + getSubsegmentsLock().unlock(); + } } } @@ -756,7 +758,12 @@ public String prettySerialize() { @Override public void removeSubsegment(Subsegment subsegment) { synchronized (lock) { - subsegments.remove(subsegment); + getSubsegmentsLock().lock(); + try { + subsegments.remove(subsegment); + } finally { + getSubsegmentsLock().unlock(); + } getParentSegment().getTotalSize().decrement(); } } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java index 8dd6baa2..6e2a2c92 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java @@ -282,11 +282,6 @@ public List getSubsegments() { return NoOpList.get(); } - @Override - public List getSubsegmentsCopy() { - return NoOpList.get(); - } - @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java index 1f10fd80..d9d67601 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java @@ -231,11 +231,6 @@ public List getSubsegments() { return NoOpList.get(); } - @Override - public List getSubsegmentsCopy() { - return NoOpList.get(); - } - @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java index 8ec734c5..fc289d19 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java @@ -20,7 +20,6 @@ import com.amazonaws.xray.entities.Segment; import com.amazonaws.xray.entities.Subsegment; import java.util.ArrayList; -import java.util.List; public class DefaultStreamingStrategy implements StreamingStrategy { @@ -83,18 +82,30 @@ public boolean requiresStreaming(Segment segment) { */ @Override public void streamSome(Entity entity, Emitter emitter) { - stream(entity, emitter); + if (entity.getSubsegmentsLock().tryLock()) { + try { + stream(entity, emitter); + } finally { + entity.getSubsegmentsLock().unlock(); + } + } } private boolean stream(Entity entity, Emitter emitter) { - List children = entity.getSubsegmentsCopy(); - List streamable = new ArrayList<>(); + ArrayList children = new ArrayList<>(entity.getSubsegments()); + ArrayList streamable = new ArrayList<>(); //Gather children and in the condition they are ready to stream, add them to the streamable list. if (children.size() > 0) { for (Subsegment child : children) { - if (stream(child, emitter)) { - streamable.add(child); + if (child.getSubsegmentsLock().tryLock()) { + try { + if (stream(child, emitter)) { + streamable.add(child); + } + } finally { + child.getSubsegmentsLock().unlock(); + } } } } From 61e317371bee608c4ab6b68fb6f693feec08dbdd Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:15:45 -0700 Subject: [PATCH 4/9] Revert "Lock all accesses to entities. (#250)" This reverts commit f0a3b3a7d78e8606f0f7b18cba4be91509e7a385. --- .../amazonaws/xray/entities/EntityImpl.java | 454 ++++++------------ .../xray/entities/FacadeSegment.java | 6 + .../amazonaws/xray/entities/SegmentImpl.java | 115 ++--- .../xray/entities/SubsegmentImpl.java | 114 ++--- build.gradle.kts | 2 - 5 files changed, 244 insertions(+), 447 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java index aecc41a0..1a2e6e73 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java @@ -34,11 +34,10 @@ import com.fasterxml.jackson.databind.node.NullNode; import com.fasterxml.jackson.databind.ser.BeanSerializerModifier; import com.fasterxml.jackson.databind.ser.std.ToStringSerializer; -import com.google.errorprone.annotations.concurrent.GuardedBy; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.LongAdder; import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; @@ -67,81 +66,59 @@ public abstract class EntityImpl implements Entity { private static final String DEFAULT_METADATA_NAMESPACE = "default"; - protected final Object lock = new Object(); - private final String name; - /* * Reference counter to track how many subsegments are in progress on this entity. Starts with a value of 0. */ @JsonIgnore - protected int referenceCount; + protected LongAdder referenceCount; @JsonIgnore protected LongAdder totalSize; - @GuardedBy("lock") + private String name; private String id; - @GuardedBy("lock") @Nullable private String parentId; - @GuardedBy("lock") private double startTime; @JsonInclude(Include.NON_DEFAULT) @JsonSerialize(using = ToStringSerializer.class) - @GuardedBy("lock") private TraceID traceId; @JsonInclude(Include.NON_DEFAULT) - @GuardedBy("lock") private double endTime; @JsonInclude(Include.NON_DEFAULT) - @GuardedBy("lock") private boolean fault; @JsonInclude(Include.NON_DEFAULT) - @GuardedBy("lock") private boolean error; @JsonInclude(Include.NON_DEFAULT) - @GuardedBy("lock") private boolean throttle; @JsonInclude(Include.NON_DEFAULT) - @GuardedBy("lock") private boolean inProgress; @Nullable - @GuardedBy("lock") private String namespace; // TODO(anuraaga): Check final for other variables, for now this is most important since it's also a lock. private final List subsegments; - @GuardedBy("lock") private Cause cause; - @GuardedBy("lock") private Map http; - @GuardedBy("lock") private Map aws; - @GuardedBy("lock") private Map sql; - @GuardedBy("lock") private Map> metadata; - @GuardedBy("lock") private Map annotations; @JsonIgnore - @GuardedBy("lock") private Entity parent; @JsonIgnore - @GuardedBy("lock") private AWSXRayRecorder creator; @JsonIgnore - @GuardedBy("lock") private ReentrantLock subsegmentsLock; @JsonIgnore - @GuardedBy("lock") private boolean emitted = false; static { @@ -181,7 +158,6 @@ public JsonSerializer modifySerializer( protected EntityImpl() { // TODO(anuraaga): Check this is working as intended, empty lists are currently serialized. subsegments = null; - name = null; } // TODO(anuraaga): Refactor the entity relationship. There isn't a great reason to use a type hierarchy for data classes and @@ -196,15 +172,15 @@ protected EntityImpl(AWSXRayRecorder creator, String name) { this.subsegments = new ArrayList<>(); this.subsegmentsLock = new ReentrantLock(); this.cause = new Cause(); - this.http = new HashMap<>(); - this.aws = new HashMap<>(); - this.sql = new HashMap<>(); - this.annotations = new HashMap<>(); - this.metadata = new HashMap<>(); + this.http = new ConcurrentHashMap<>(); + this.aws = new ConcurrentHashMap<>(); + this.sql = new ConcurrentHashMap<>(); + this.annotations = new ConcurrentHashMap<>(); + this.metadata = new ConcurrentHashMap<>(); this.startTime = System.currentTimeMillis() / 1000d; this.id = creator.getIdGenerator().newEntityId(); this.inProgress = true; - this.referenceCount = 0; + this.referenceCount = new LongAdder(); this.totalSize = new LongAdder(); } @@ -217,11 +193,9 @@ protected EntityImpl(AWSXRayRecorder creator, String name) { * */ protected void checkAlreadyEmitted() { - synchronized (lock) { - if (emitted) { - getCreator().getContextMissingStrategy().contextMissing("Segment " + getName() + " has already been emitted.", - AlreadyEmittedException.class); - } + if (emitted) { + getCreator().getContextMissingStrategy().contextMissing("Segment " + getName() + " has already been emitted.", + AlreadyEmittedException.class); } } @@ -232,205 +206,151 @@ public String getName() { @Override public String getId() { - synchronized (lock) { - return id; - } + return id; } @Override public void setId(String id) { - synchronized (lock) { - checkAlreadyEmitted(); - this.id = id; - } + checkAlreadyEmitted(); + this.id = id; } @Override public double getStartTime() { - synchronized (lock) { - return startTime; - } + return startTime; } @Override public void setStartTime(double startTime) { - synchronized (lock) { - checkAlreadyEmitted(); - this.startTime = startTime; - } + checkAlreadyEmitted(); + this.startTime = startTime; } @Override public double getEndTime() { - synchronized (lock) { - return endTime; - } + return endTime; } @Override public void setEndTime(double endTime) { - synchronized (lock) { - checkAlreadyEmitted(); - this.endTime = endTime; - } + checkAlreadyEmitted(); + this.endTime = endTime; } @Override public boolean isFault() { - synchronized (lock) { - return fault; - } + return fault; } @Override public void setFault(boolean fault) { - synchronized (lock) { - checkAlreadyEmitted(); - this.fault = fault; - } + checkAlreadyEmitted(); + this.fault = fault; } @Override public boolean isError() { - synchronized (lock) { - return error; - } + return error; } @Override public void setError(boolean error) { - synchronized (lock) { - checkAlreadyEmitted(); - this.error = error; - } + checkAlreadyEmitted(); + this.error = error; } @Override @Nullable public String getNamespace() { - synchronized (lock) { - return namespace; - } + return namespace; } @Override public void setNamespace(String namespace) { - synchronized (lock) { - checkAlreadyEmitted(); - this.namespace = namespace; - } + checkAlreadyEmitted(); + this.namespace = namespace; } @Override public ReentrantLock getSubsegmentsLock() { - synchronized (lock) { - return subsegmentsLock; - } + return subsegmentsLock; } @Override public void setSubsegmentsLock(ReentrantLock subsegmentsLock) { - synchronized (lock) { - checkAlreadyEmitted(); - this.subsegmentsLock = subsegmentsLock; - } + checkAlreadyEmitted(); + this.subsegmentsLock = subsegmentsLock; } @Override public Cause getCause() { - synchronized (lock) { - return cause; - } + return cause; } @Override public Map getHttp() { - synchronized (lock) { - return http; - } + return http; } @Override public void setHttp(Map http) { - synchronized (lock) { - checkAlreadyEmitted(); - this.http = http; - } + checkAlreadyEmitted(); + this.http = http; } @Override public Map getAws() { - synchronized (lock) { - return aws; - } + return aws; } @Override public void setAws(Map aws) { - synchronized (lock) { - checkAlreadyEmitted(); - this.aws = aws; - } + checkAlreadyEmitted(); + this.aws = aws; } @Override public Map getSql() { - synchronized (lock) { - return sql; - } + return sql; } @Override public void setSql(Map sql) { - synchronized (lock) { - checkAlreadyEmitted(); - this.sql = sql; - } + checkAlreadyEmitted(); + this.sql = sql; } @Override public Map> getMetadata() { - synchronized (lock) { - return metadata; - } + return metadata; } @Override public void setMetadata(Map> metadata) { - synchronized (lock) { - checkAlreadyEmitted(); - this.metadata = metadata; - } + checkAlreadyEmitted(); + this.metadata = metadata; } @Override public Map getAnnotations() { - synchronized (lock) { - return annotations; - } + return annotations; } @Override public void setAnnotations(Map annotations) { - synchronized (lock) { - checkAlreadyEmitted(); - this.annotations = annotations; - } + checkAlreadyEmitted(); + this.annotations = annotations; } @Override public Entity getParent() { - synchronized (lock) { - return parent; - } + return parent; } @Override public void setParent(Entity parent) { - synchronized (lock) { - checkAlreadyEmitted(); - this.parent = parent; - } + checkAlreadyEmitted(); + this.parent = parent; } /** @@ -438,9 +358,7 @@ public void setParent(Entity parent) { */ @Override public AWSXRayRecorder getCreator() { - synchronized (lock) { - return creator; - } + return creator; } /** @@ -448,76 +366,58 @@ public AWSXRayRecorder getCreator() { */ @Override public void setCreator(AWSXRayRecorder creator) { - synchronized (lock) { - checkAlreadyEmitted(); - this.creator = creator; - } + checkAlreadyEmitted(); + this.creator = creator; } @Override public boolean isThrottle() { - synchronized (lock) { - return throttle; - } + return throttle; } @Override public void setThrottle(boolean throttle) { - synchronized (lock) { - checkAlreadyEmitted(); - if (throttle) { - this.fault = false; - this.error = true; - } - this.throttle = throttle; + checkAlreadyEmitted(); + if (throttle) { + this.fault = false; + this.error = true; } + this.throttle = throttle; } @Override public boolean isInProgress() { - synchronized (lock) { - return inProgress; - } + return inProgress; } @Override public void setInProgress(boolean inProgress) { - synchronized (lock) { - checkAlreadyEmitted(); - this.inProgress = inProgress; - } + checkAlreadyEmitted(); + this.inProgress = inProgress; } @Override public TraceID getTraceId() { - synchronized (lock) { - return traceId; - } + return traceId; } @Override @EnsuresNonNull("this.traceId") public void setTraceId(TraceID traceId) { - synchronized (lock) { - checkAlreadyEmitted(); - this.traceId = traceId; - } + checkAlreadyEmitted(); + this.traceId = traceId; } @Override @Nullable public String getParentId() { - synchronized (lock) { - return parentId; - } + return parentId; } @Override public void setParentId(@Nullable String parentId) { - synchronized (lock) { - checkAlreadyEmitted(); - this.parentId = parentId; - } + checkAlreadyEmitted(); + this.parentId = parentId; } @@ -527,163 +427,133 @@ public void setParentId(@Nullable String parentId) { @Override public List getSubsegments() { - synchronized (lock) { - return subsegments; - } + return subsegments; } @Override public void addSubsegment(Subsegment subsegment) { - synchronized (lock) { - checkAlreadyEmitted(); - getSubsegmentsLock().lock(); - try { - subsegments.add(subsegment); - } finally { - getSubsegmentsLock().unlock(); - } + checkAlreadyEmitted(); + getSubsegmentsLock().lock(); + try { + subsegments.add(subsegment); + } finally { + getSubsegmentsLock().unlock(); } } @Override public void addException(Throwable exception) { - synchronized (lock) { - checkAlreadyEmitted(); - setFault(true); - getSubsegmentsLock().lock(); - try { - cause.addExceptions(creator.getThrowableSerializationStrategy() - .describeInContext(this, exception, subsegments)); - } finally { - getSubsegmentsLock().unlock(); - } + checkAlreadyEmitted(); + setFault(true); + getSubsegmentsLock().lock(); + try { + cause.addExceptions(creator.getThrowableSerializationStrategy() + .describeInContext(this, exception, subsegments)); + } finally { + getSubsegmentsLock().unlock(); } } @Override public void putHttp(String key, Object value) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(key); - validateNotNull(value); - http.put(key, value); - } + checkAlreadyEmitted(); + validateNotNull(key); + validateNotNull(value); + http.put(key, value); } @Override public void putAllHttp(Map all) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(all); - http.putAll(all); - } + checkAlreadyEmitted(); + validateNotNull(all); + http.putAll(all); } @Override public void putAws(String key, Object value) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(key); - validateNotNull(value); - aws.put(key, value); - } + checkAlreadyEmitted(); + validateNotNull(key); + validateNotNull(value); + aws.put(key, value); } @Override public void putAllAws(Map all) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(all); - aws.putAll(all); - } + checkAlreadyEmitted(); + validateNotNull(all); + aws.putAll(all); } @Override public void putSql(String key, Object value) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(key); - validateNotNull(value); - sql.put(key, value); - } + checkAlreadyEmitted(); + validateNotNull(key); + validateNotNull(value); + sql.put(key, value); } @Override public void putAllSql(Map all) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(all); - sql.putAll(all); - } + checkAlreadyEmitted(); + validateNotNull(all); + sql.putAll(all); } @Override public void putAnnotation(String key, String value) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(key); - validateNotNull(value); - annotations.put(key, value); - } + checkAlreadyEmitted(); + validateNotNull(key); + validateNotNull(value); + annotations.put(key, value); } @Override public void putAnnotation(String key, Number value) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(key); - validateNotNull(value); - annotations.put(key, value); - } + checkAlreadyEmitted(); + validateNotNull(key); + validateNotNull(value); + annotations.put(key, value); } @Override public void putAnnotation(String key, Boolean value) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(key); - validateNotNull(value); - annotations.put(key, value); - } + checkAlreadyEmitted(); + validateNotNull(key); + validateNotNull(value); + annotations.put(key, value); } @Override public void putMetadata(String key, Object object) { - synchronized (lock) { - checkAlreadyEmitted(); - putMetadata(DEFAULT_METADATA_NAMESPACE, key, object); - } + checkAlreadyEmitted(); + putMetadata(DEFAULT_METADATA_NAMESPACE, key, object); } @Override public void putMetadata(String namespace, String key, Object object) { - synchronized (lock) { - checkAlreadyEmitted(); - validateNotNull(namespace); - validateNotNull(key); - if (null == object) { - object = NullNode.instance; - } - metadata.computeIfAbsent(namespace, unused -> new HashMap<>()).put(key, object); + checkAlreadyEmitted(); + validateNotNull(namespace); + validateNotNull(key); + if (null == object) { + object = NullNode.instance; } + metadata.computeIfAbsent(namespace, (n) -> { + return new ConcurrentHashMap(); + }).put(key, object); } @Override public void incrementReferenceCount() { - synchronized (lock) { - checkAlreadyEmitted(); - referenceCount++; - totalSize.increment(); - } + checkAlreadyEmitted(); + referenceCount.increment(); + totalSize.increment(); } @Override public boolean decrementReferenceCount() { - synchronized (lock) { - checkAlreadyEmitted(); - referenceCount--; - return !isInProgress() && referenceCount <= 0; - } + checkAlreadyEmitted(); + referenceCount.decrement(); + return !isInProgress() && referenceCount.intValue() <= 0; } /** @@ -694,9 +564,7 @@ public boolean decrementReferenceCount() { */ @Override public int getReferenceCount() { - synchronized (lock) { - return referenceCount; - } + return referenceCount.intValue(); } /** @@ -704,9 +572,7 @@ public int getReferenceCount() { */ @Override public LongAdder getTotalSize() { - synchronized (lock) { - return totalSize; - } + return totalSize; } /** @@ -714,9 +580,7 @@ public LongAdder getTotalSize() { */ @Override public boolean isEmitted() { - synchronized (lock) { - return emitted; - } + return emitted; } /** @@ -725,47 +589,39 @@ public boolean isEmitted() { */ @Override public void setEmitted(boolean emitted) { - synchronized (lock) { - checkAlreadyEmitted(); - this.emitted = emitted; - } + checkAlreadyEmitted(); + this.emitted = emitted; } @Override public String serialize() { - synchronized (lock) { - try { - return mapper.writeValueAsString(this); - } catch (JsonProcessingException jpe) { - logger.error("Exception while serializing entity.", jpe); - } - return ""; + try { + return mapper.writeValueAsString(this); + } catch (JsonProcessingException jpe) { + logger.error("Exception while serializing entity.", jpe); } + return ""; } @Override public String prettySerialize() { - synchronized (lock) { - try { - return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(this); - } catch (JsonProcessingException jpe) { - logger.error("Exception while serializing segment.", jpe); - } - return ""; + try { + return mapper.writerWithDefaultPrettyPrinter().writeValueAsString(this); + } catch (JsonProcessingException jpe) { + logger.error("Exception while serializing segment.", jpe); } + return ""; } @Override public void removeSubsegment(Subsegment subsegment) { - synchronized (lock) { - getSubsegmentsLock().lock(); - try { - subsegments.remove(subsegment); - } finally { - getSubsegmentsLock().unlock(); - } - getParentSegment().getTotalSize().decrement(); + getSubsegmentsLock().lock(); + try { + subsegments.remove(subsegment); + } finally { + getSubsegmentsLock().unlock(); } + getParentSegment().getTotalSize().decrement(); } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/FacadeSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/FacadeSegment.java index 2e6815bf..a20c6e37 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/FacadeSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/FacadeSegment.java @@ -26,6 +26,12 @@ public class FacadeSegment extends EntityImpl implements Segment { private static final String MUTATION_UNSUPPORTED_MESSAGE = "FacadeSegments cannot be mutated."; private static final String INFORMATION_UNAVAILABLE_MESSAGE = "This information is unavailable."; + protected String resourceArn; + protected String user; + protected String origin; + + protected Map service; + private final boolean sampled; // TODO(anuraaga): Refactor the entity relationship. There isn't a great reason to use a type hierarchy for data classes and diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java index 6a23b461..4cc9209d 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SegmentImpl.java @@ -17,24 +17,19 @@ import com.amazonaws.xray.AWSXRayRecorder; import com.fasterxml.jackson.annotation.JsonIgnore; -import com.google.errorprone.annotations.concurrent.GuardedBy; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; public class SegmentImpl extends EntityImpl implements Segment { - @GuardedBy("lock") protected String resourceArn; - @GuardedBy("lock") protected String user; - @GuardedBy("lock") protected String origin; - @GuardedBy("lock") protected Map service; @JsonIgnore - @GuardedBy("lock") private boolean sampled; @SuppressWarnings({ "unused", "nullness" }) @@ -59,26 +54,24 @@ public SegmentImpl(AWSXRayRecorder creator, String name, TraceID traceId) { } setTraceId(traceId); - this.service = new HashMap<>(); + this.service = new ConcurrentHashMap<>(); this.sampled = true; } @Override public boolean end() { - synchronized (lock) { - if (getEndTime() < Double.MIN_NORMAL) { - setEndTime(System.currentTimeMillis() / 1000d); - } + if (getEndTime() < Double.MIN_NORMAL) { + setEndTime(System.currentTimeMillis() / 1000d); + } - setInProgress(false); - boolean shouldEmit = referenceCount <= 0; - if (shouldEmit) { - checkAlreadyEmitted(); - setEmitted(true); - } - return shouldEmit; + setInProgress(false); + boolean shouldEmit = referenceCount.intValue() <= 0; + if (shouldEmit) { + checkAlreadyEmitted(); + setEmitted(true); } + return shouldEmit; } @Override @@ -88,109 +81,83 @@ public boolean isRecording() { @Override public boolean isSampled() { - synchronized (lock) { - return sampled; - } + return sampled; } @Override public void setSampled(boolean sampled) { - synchronized (lock) { - checkAlreadyEmitted(); - this.sampled = sampled; - } + checkAlreadyEmitted(); + this.sampled = sampled; } @Override public String getResourceArn() { - synchronized (lock) { - return resourceArn; - } + return resourceArn; } @Override public void setResourceArn(String resourceArn) { - synchronized (lock) { - checkAlreadyEmitted(); - this.resourceArn = resourceArn; - } + checkAlreadyEmitted(); + this.resourceArn = resourceArn; } @Override public String getUser() { - synchronized (lock) { - return user; - } + return user; } @Override public void setUser(String user) { - synchronized (lock) { - checkAlreadyEmitted(); - this.user = user; - } + checkAlreadyEmitted(); + this.user = user; } @Override public String getOrigin() { - synchronized (lock) { - return origin; - } + return origin; } @Override public void setOrigin(String origin) { - synchronized (lock) { - checkAlreadyEmitted(); - this.origin = origin; - } + checkAlreadyEmitted(); + this.origin = origin; } @Override public Map getService() { - synchronized (lock) { - return service; - } + return service; } @Override public void setService(Map service) { - synchronized (lock) { - checkAlreadyEmitted(); - this.service = service; - } + checkAlreadyEmitted(); + this.service = service; } @Override public void putService(String key, Object object) { - synchronized (lock) { - checkAlreadyEmitted(); - service.put(key, object); - } + checkAlreadyEmitted(); + service.put(key, object); } @Override public void putAllService(Map all) { - synchronized (lock) { - checkAlreadyEmitted(); - service.putAll(all); - } + checkAlreadyEmitted(); + service.putAll(all); } @Override public void setRuleName(String ruleName) { - synchronized (lock) { - checkAlreadyEmitted(); - if (getAws().get("xray") instanceof Map) { - @SuppressWarnings("unchecked") - Map a = (Map) getAws().get("xray"); - HashMap referA = new HashMap<>(); - if (a != null) { - referA.putAll(a); - } - referA.put("rule_name", ruleName); - this.putAws("xray", referA); + checkAlreadyEmitted(); + if (getAws().get("xray") instanceof Map) { + @SuppressWarnings("unchecked") + Map a = (Map) getAws().get("xray"); + HashMap referA = new HashMap<>(); + if (a != null) { + referA.putAll(a); } + referA.put("rule_name", ruleName); + this.putAws("xray", referA); } } @@ -201,9 +168,7 @@ public Segment getParentSegment() { @Override public void close() { - synchronized (lock) { - getCreator().endSegment(); - } + getCreator().endSegment(); } } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java index b8a2cb9d..f02ff2eb 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java @@ -18,7 +18,6 @@ import com.amazonaws.xray.AWSXRayRecorder; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.node.ObjectNode; -import com.google.errorprone.annotations.concurrent.GuardedBy; import java.util.HashSet; import java.util.Set; import org.apache.commons.logging.Log; @@ -29,13 +28,10 @@ public class SubsegmentImpl extends EntityImpl implements Subsegment { private static final Log logger = LogFactory.getLog(SubsegmentImpl.class); @Nullable - @GuardedBy("lock") private String namespace; - @GuardedBy("lock") private Segment parentSegment; - @GuardedBy("lock") private Set precursorIds; @GuardedBy("lock") @@ -56,77 +52,61 @@ public SubsegmentImpl(AWSXRayRecorder creator, String name, Segment parentSegmen @Override public boolean end() { - synchronized (lock) { - if (logger.isDebugEnabled()) { - logger.debug("Subsegment named '" + getName() + "' ending. Parent segment named '" + parentSegment.getName() - + "' has reference count " + parentSegment.getReferenceCount()); - } - - if (getEndTime() < Double.MIN_NORMAL) { - setEndTime(System.currentTimeMillis() / 1000d); - } - setInProgress(false); - boolean shouldEmit = parentSegment.decrementReferenceCount() && parentSegment.isSampled(); - if (shouldEmit) { - checkAlreadyEmitted(); - setEmitted(true); - } - return shouldEmit; + if (logger.isDebugEnabled()) { + logger.debug("Subsegment named '" + getName() + "' ending. Parent segment named '" + parentSegment.getName() + + "' has reference count " + parentSegment.getReferenceCount()); } + + if (getEndTime() < Double.MIN_NORMAL) { + setEndTime(System.currentTimeMillis() / 1000d); + } + setInProgress(false); + boolean shouldEmit = parentSegment.decrementReferenceCount() && parentSegment.isSampled(); + if (shouldEmit) { + checkAlreadyEmitted(); + setEmitted(true); + } + return shouldEmit; } @Override @Nullable public String getNamespace() { - synchronized (lock) { - return namespace; - } + return namespace; } @Override public void setNamespace(String namespace) { - synchronized (lock) { - checkAlreadyEmitted(); - this.namespace = namespace; - } + checkAlreadyEmitted(); + this.namespace = namespace; } @Override public Segment getParentSegment() { - synchronized (lock) { - return parentSegment; - } + return parentSegment; } @Override public void setParentSegment(Segment parentSegment) { - synchronized (lock) { - checkAlreadyEmitted(); - this.parentSegment = parentSegment; - } + checkAlreadyEmitted(); + this.parentSegment = parentSegment; } @Override public void addPrecursorId(String precursorId) { - synchronized (lock) { - checkAlreadyEmitted(); - this.precursorIds.add(precursorId); - } + checkAlreadyEmitted(); + this.precursorIds.add(precursorId); } @Override public Set getPrecursorIds() { - synchronized (lock) { - return precursorIds; - } + return precursorIds; } @Override public void setPrecursorIds(Set precursorIds) { - synchronized (lock) { - checkAlreadyEmitted(); - this.precursorIds = precursorIds; - } + checkAlreadyEmitted(); + this.precursorIds = precursorIds; } @Override @@ -137,45 +117,37 @@ public boolean shouldPropagate() { } private ObjectNode getStreamSerializeObjectNode() { - synchronized (lock) { - ObjectNode obj = (ObjectNode) mapper.valueToTree(this); - obj.put("type", "subsegment"); - obj.put("parent_id", getParent().getId()); - obj.put("trace_id", parentSegment.getTraceId().toString()); - return obj; - } + ObjectNode obj = (ObjectNode) mapper.valueToTree(this); + obj.put("type", "subsegment"); + obj.put("parent_id", getParent().getId()); + obj.put("trace_id", parentSegment.getTraceId().toString()); + return obj; } @Override public String streamSerialize() { - synchronized (lock) { - String ret = ""; - try { - ret = mapper.writeValueAsString(getStreamSerializeObjectNode()); - } catch (JsonProcessingException jpe) { - logger.error("Exception while serializing entity.", jpe); - } - return ret; + String ret = ""; + try { + ret = mapper.writeValueAsString(getStreamSerializeObjectNode()); + } catch (JsonProcessingException jpe) { + logger.error("Exception while serializing entity.", jpe); } + return ret; } @Override public String prettyStreamSerialize() { - synchronized (lock) { - String ret = ""; - try { - ret = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(getStreamSerializeObjectNode()); - } catch (JsonProcessingException jpe) { - logger.error("Exception while serializing entity.", jpe); - } - return ret; + String ret = ""; + try { + ret = mapper.writerWithDefaultPrettyPrinter().writeValueAsString(getStreamSerializeObjectNode()); + } catch (JsonProcessingException jpe) { + logger.error("Exception while serializing entity.", jpe); } + return ret; } @Override public void close() { - synchronized (lock) { - getCreator().endSubsegment(); - } + getCreator().endSubsegment(); } } diff --git a/build.gradle.kts b/build.gradle.kts index d6e1d6d4..d7110e07 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -133,8 +133,6 @@ allprojects { add("testImplementation", "org.mockito:mockito-core") add("testImplementation", "org.mockito:mockito-junit-jupiter") - add("compileOnly", "com.google.errorprone:error_prone_annotations") - add("compileOnly", "org.checkerframework:checker-qual:3.4.1") add("testImplementation", "org.checkerframework:checker-qual:3.4.1") add("checkerFramework", "org.checkerframework:checker:3.4.1") From 766a1312bfca98ded2f1eb80f2e41e96b695fb7e Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:16:44 -0700 Subject: [PATCH 5/9] add back errorprone annotations --- build.gradle.kts | 1 + 1 file changed, 1 insertion(+) diff --git a/build.gradle.kts b/build.gradle.kts index d7110e07..37001cdf 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -133,6 +133,7 @@ allprojects { add("testImplementation", "org.mockito:mockito-core") add("testImplementation", "org.mockito:mockito-junit-jupiter") + add("compileOnly", "com.google.errorprone:error_prone_annotations") add("compileOnly", "org.checkerframework:checker-qual:3.4.1") add("testImplementation", "org.checkerframework:checker-qual:3.4.1") add("checkerFramework", "org.checkerframework:checker:3.4.1") From e20fabf889b2d11447667c041dc729c7ac9d9e40 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:22:23 -0700 Subject: [PATCH 6/9] fixed shouldPropagate --- .../java/com/amazonaws/xray/entities/SubsegmentImpl.java | 5 +---- build.gradle.kts | 1 - dependencyManagement/build.gradle.kts | 5 ----- 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java index f02ff2eb..607862e7 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/SubsegmentImpl.java @@ -34,7 +34,6 @@ public class SubsegmentImpl extends EntityImpl implements Subsegment { private Set precursorIds; - @GuardedBy("lock") private boolean shouldPropagate; @SuppressWarnings("nullness") @@ -111,9 +110,7 @@ public void setPrecursorIds(Set precursorIds) { @Override public boolean shouldPropagate() { - synchronized (lock) { - return shouldPropagate; - } + return shouldPropagate; } private ObjectNode getStreamSerializeObjectNode() { diff --git a/build.gradle.kts b/build.gradle.kts index 37001cdf..d7110e07 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -133,7 +133,6 @@ allprojects { add("testImplementation", "org.mockito:mockito-core") add("testImplementation", "org.mockito:mockito-junit-jupiter") - add("compileOnly", "com.google.errorprone:error_prone_annotations") add("compileOnly", "org.checkerframework:checker-qual:3.4.1") add("testImplementation", "org.checkerframework:checker-qual:3.4.1") add("checkerFramework", "org.checkerframework:checker:3.4.1") diff --git a/dependencyManagement/build.gradle.kts b/dependencyManagement/build.gradle.kts index 4e656612..daf701e9 100644 --- a/dependencyManagement/build.gradle.kts +++ b/dependencyManagement/build.gradle.kts @@ -10,11 +10,6 @@ val DEPENDENCY_BOMS = listOf( ) val DEPENDENCY_SETS = listOf( - DependencySet( - "com.google.errorprone", - "2.5.1", - listOf("error_prone_annotations") - ), DependencySet( "com.fasterxml.jackson.datatype", "2.12.0", From 4a40b2dee31ad8785c86e2b7ce75238613c00598 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:39:53 -0700 Subject: [PATCH 7/9] added back getSubsegmentsCopy --- .../java/com/amazonaws/xray/entities/DummySegment.java | 6 ++++++ .../com/amazonaws/xray/entities/DummySubsegment.java | 6 ++++++ .../main/java/com/amazonaws/xray/entities/Entity.java | 9 +++++++++ .../java/com/amazonaws/xray/entities/EntityImpl.java | 6 ++++++ .../java/com/amazonaws/xray/entities/NoOpSegment.java | 5 +++++ .../java/com/amazonaws/xray/entities/NoOpSubSegment.java | 5 +++++ 6 files changed, 37 insertions(+) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java index 9c8afc85..f61e9eab 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java @@ -241,6 +241,12 @@ public List getSubsegments() { return list; } + @Override + public List getSubsegmentsCopy() { + return new ArrayList<>(list); + } + + @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java index ee29f2f5..dc8fdd16 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java @@ -223,6 +223,12 @@ public List getSubsegments() { return list; } + @Override + public List getSubsegmentsCopy() { + return new ArrayList<>(list); + } + + @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java index 3cbc81ed..0a410070 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java @@ -367,9 +367,18 @@ default void run(Runnable runnable, AWSXRayRecorder recorder) { /** * @return the subsegments + * + * @deprecated Use {@link #getSubsegmentsCopy()}. */ + @Deprecated List getSubsegments(); + /** + * Returns a copy of the currently added subsegments. Updates to the returned {@link List} will not be reflected in the + * {@link Entity}. + */ + List getSubsegmentsCopy(); + /** * Adds a subsegment. * diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java index 1a2e6e73..11ead309 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java @@ -430,6 +430,12 @@ public List getSubsegments() { return subsegments; } + @JsonIgnore + @Override + public List getSubsegmentsCopy() { + return new ArrayList<>(subsegments); + } + @Override public void addSubsegment(Subsegment subsegment) { checkAlreadyEmitted(); diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java index 6e2a2c92..8dd6baa2 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java @@ -282,6 +282,11 @@ public List getSubsegments() { return NoOpList.get(); } + @Override + public List getSubsegmentsCopy() { + return NoOpList.get(); + } + @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java index d9d67601..1f10fd80 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java @@ -231,6 +231,11 @@ public List getSubsegments() { return NoOpList.get(); } + @Override + public List getSubsegmentsCopy() { + return NoOpList.get(); + } + @Override public void addSubsegment(Subsegment subsegment) { } From 63bdaf364b6a18a85458fa990ae6eec7cf28e270 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:48:42 -0700 Subject: [PATCH 8/9] added back compareAndSetEmitted --- .../com/amazonaws/xray/entities/DummySegment.java | 5 +++++ .../com/amazonaws/xray/entities/DummySubsegment.java | 5 +++++ .../java/com/amazonaws/xray/entities/Entity.java | 12 ++++++++++++ .../java/com/amazonaws/xray/entities/EntityImpl.java | 10 ++++++++++ .../com/amazonaws/xray/entities/NoOpSegment.java | 5 +++++ .../com/amazonaws/xray/entities/NoOpSubSegment.java | 5 +++++ 6 files changed, 42 insertions(+) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java index f61e9eab..51a134ec 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java @@ -308,6 +308,11 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } + @Override + public boolean compareAndSetEmitted(boolean current, boolean next) { + return false; + } + @Override public String serialize() { return ""; diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java index dc8fdd16..24529e8b 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java @@ -290,6 +290,11 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } + @Override + public boolean compareAndSetEmitted(boolean current, boolean next) { + return false; + } + @Override public String serialize() { return ""; diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java index 0a410070..d067c0ec 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java @@ -600,8 +600,20 @@ default void run(Runnable runnable, AWSXRayRecorder recorder) { boolean isEmitted(); + /** + * Sets emitted on the entity. + */ void setEmitted(boolean emitted); + /** + * Checks whether this {@link Entity} currently has emitted state of {@code current} and if so, set emitted state to + * {@code next}. Returns {@code true} if the state was updated, or {@code false} otherwise. + * + * @deprecated Use {@link #setEmitted(boolean)} + */ + @Deprecated + boolean compareAndSetEmitted(boolean current, boolean next); + String serialize(); String prettySerialize(); diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java index 11ead309..7591deec 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java @@ -599,6 +599,16 @@ public void setEmitted(boolean emitted) { this.emitted = emitted; } + @Override + public boolean compareAndSetEmitted(boolean current, boolean next) { + checkAlreadyEmitted(); + if (emitted == current) { + emitted = next; + return true; + } + return false; + } + @Override public String serialize() { try { diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java index 8dd6baa2..e406e9ac 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java @@ -372,6 +372,11 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } + @Override + public boolean compareAndSetEmitted(boolean current, boolean next) { + return false; + } + @Override public String serialize() { return ""; diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java index 1f10fd80..ca82b064 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java @@ -321,6 +321,11 @@ public boolean isEmitted() { public void setEmitted(boolean emitted) { } + @Override + public boolean compareAndSetEmitted(boolean current, boolean next) { + return false; + } + @Override public String serialize() { return ""; From c581f5ee5d1bb22e1f0b720d9a2fad3968a26af5 Mon Sep 17 00:00:00 2001 From: William Armiros Date: Sat, 13 Aug 2022 13:52:40 -0700 Subject: [PATCH 9/9] whitespace fix --- .../src/main/java/com/amazonaws/xray/entities/DummySegment.java | 1 - .../main/java/com/amazonaws/xray/entities/DummySubsegment.java | 1 - 2 files changed, 2 deletions(-) diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java index 51a134ec..d1c0961e 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java @@ -246,7 +246,6 @@ public List getSubsegmentsCopy() { return new ArrayList<>(list); } - @Override public void addSubsegment(Subsegment subsegment) { } diff --git a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java index 24529e8b..16bf12ef 100644 --- a/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java +++ b/aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java @@ -228,7 +228,6 @@ public List getSubsegmentsCopy() { return new ArrayList<>(list); } - @Override public void addSubsegment(Subsegment subsegment) { }