Optimize SpanBuilderSdk link memory (#753)

* ignore links beyond the limit, rather than waiting until the end to truncate on span creation. * update for changes from upstream * remove the todo; replaced with #766
open-telemetry · Jan 17, 2020 · 564a323 · 564a323
1 parent 61b50a6
commit 564a323
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 10 deletions.
diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanBuilderSdk.java
@@ -62,6 +62,7 @@ class SpanBuilderSdk implements Span.Builder {
   private Kind spanKind = Kind.INTERNAL;
   private final AttributesWithCapacity attributes;
   private List<Link> links;
+  private int totalNumberOfLinksAdded = 0;
   private ParentType parentType = ParentType.CURRENT_SPAN;
   private long startEpochNanos = 0;
 
@@ -129,10 +130,17 @@ public Span.Builder addLink(SpanContext spanContext, Map<String, AttributeValue>
   @Override
   public Span.Builder addLink(Link link) {
     Utils.checkNotNull(link, "link");
+    totalNumberOfLinksAdded++;
+    // don't bother doing anything with any links beyond the max.
+    if (links.size() == traceConfig.getMaxNumberOfLinks()) {
+      return this;
+    }
+
     // This is the Collection.emptyList which is immutable.
     if (links.isEmpty()) {
       links = new ArrayList<>();
     }
+
     links.add(link);
     return this;
   }
@@ -219,18 +227,11 @@ public Span startSpan() {
         getClock(parentSpan(parentType, parent), clock),
         resource,
         attributes,
-        truncatedLinks(),
-        links.size(),
+        links,
+        totalNumberOfLinksAdded,
         startEpochNanos);
   }
 
-  private List<Link> truncatedLinks() {
-    if (links.size() <= traceConfig.getMaxNumberOfLinks()) {
-      return links;
-    }
-    return links.subList(links.size() - traceConfig.getMaxNumberOfLinks(), links.size());
-  }
-
   private static Clock getClock(Span parent, Clock clock) {
     if (parent instanceof RecordEventsReadableSpan) {
       RecordEventsReadableSpan parentRecordEventsSpan = (RecordEventsReadableSpan) parent;

diff --git a/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanData.java b/sdk/src/main/java/io/opentelemetry/sdk/trace/SpanData.java
@@ -294,11 +294,14 @@ public static Builder newBuilder() {
         .setParentSpanId(SpanId.getInvalid())
         .setInstrumentationLibraryInfo(InstrumentationLibraryInfo.EMPTY)
         .setLinks(Collections.<io.opentelemetry.trace.Link>emptyList())
+        .setTotalRecordedLinks(0)
         .setAttributes(Collections.<String, AttributeValue>emptyMap())
         .setTimedEvents(Collections.<TimedEvent>emptyList())
+        .setTotalRecordedEvents(0)
         .setResource(Resource.getEmpty())
         .setTracestate(Tracestate.getDefault())
         .setTraceFlags(TraceFlags.getDefault())
+        .setNumberOfChildren(0)
         .setHasRemoteParent(false);
   }
 

diff --git a/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java b/sdk/src/test/java/io/opentelemetry/sdk/trace/SpanBuilderSdkTest.java
@@ -110,10 +110,12 @@ public void truncateLink() {
     }
     RecordEventsReadableSpan span = (RecordEventsReadableSpan) spanBuilder.startSpan();
     try {
-      List<Link> links = span.toSpanData().getLinks();
+      SpanData spanData = span.toSpanData();
+      List<Link> links = spanData.getLinks();
       assertThat(links.size()).isEqualTo(maxNumberOfLinks);
       for (int i = 0; i < maxNumberOfLinks; i++) {
         assertThat(links.get(i)).isEqualTo(SpanData.Link.create(sampledSpanContext));
+        assertThat(spanData.getTotalRecordedLinks()).isEqualTo(2 * maxNumberOfLinks);
       }
     } finally {
       span.end();