From 564a3234da0450a95cc93e5460cc4dab496dca91 Mon Sep 17 00:00:00 2001
From: John Watson <jkwatson@gmail.com>
Date: Fri, 17 Jan 2020 15:15:14 -0800
Subject: [PATCH] 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 https://github.com/open-telemetry/opentelemetry-java/issues/766
---
 .../sdk/trace/SpanBuilderSdk.java             | 19 ++++++++++---------
 .../io/opentelemetry/sdk/trace/SpanData.java  |  3 +++
 .../sdk/trace/SpanBuilderSdkTest.java         |  4 +++-
 3 files changed, 16 insertions(+), 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
index 080829b110e..dcdfc2bf2a1 100644
--- 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
index d2f70851c48..df0f5e83348 100644
--- 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
index da0d7c05c7a..f5ff51930ef 100644
--- 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();