Makes Export methods async #1428

huntc · 2020-07-17T04:35:17Z

This PR recognises that the export and flush methods of span and trace exporters can be, and often are, implemented with long-lived operations over networks. We, therefore, see that these method return types are represented using a CompletableResultCode type that can be completed asynchronously for either failure or success scenarios. In either instance, the export code can now perform actions on failure and success. In the case of failure, which can also constitute a cancellation by an upstream component (such as a batch span processor), an exporter can now interrupt its threads or perform any other action it needs to. This approach of handling failure is also safer than what now exists, which is for an upstream component to interrupt a thread.

The PR approach is focused on changing the signatures within the export methods now, while we can still introduce API-breaking changes. The implementation of the existing exporters mostly assumes a blocking interaction as before. The remaining internal exporter behaviour can be modified in the future if required and without affecting the API. One exception to this is the OtlpGrpcMetricExporter where @anuraaga kindly updated it to become async.

For further background information on this change, see ##1422, which also refers to spec clarification via open-telemetry/opentelemetry-specification#707.

linux-foundation-easycla · 2020-07-17T04:35:21Z

The committers are authorized under a signed CLA.

✅ Christopher Hunt (79097e7)

exporters/inmemory/build.gradle

...rters/inmemory/src/main/java/io/opentelemetry/exporters/inmemory/InMemoryMetricExporter.java

...s/inmemory/src/test/java/io/opentelemetry/exporters/inmemory/InMemoryMetricExporterTest.java

exporters/logging/src/main/java/io/opentelemetry/exporters/logging/LoggingMetricExporter.java

exporters/logging/src/main/java/io/opentelemetry/exporters/logging/LoggingSpanExporter.java

codecov · 2020-07-17T05:12:13Z

Codecov Report

Merging #1428 into master will increase coverage by 0.07%.
The diff coverage is 88.81%.

@@             Coverage Diff              @@
##             master    #1428      +/-   ##
============================================
+ Coverage     86.56%   86.64%   +0.07%     
- Complexity     1367     1378      +11     
============================================
  Files           162      161       -1     
  Lines          5231     5292      +61     
  Branches        490      503      +13     
============================================
+ Hits           4528     4585      +57     
  Misses          524      524              
- Partials        179      183       +4

Impacted Files	Coverage Δ	Complexity Δ
...metry/exporters/jaeger/JaegerGrpcSpanExporter.java	`73.97% <33.33%> (ø)`	`4.00 <0.00> (ø)`
...metry/exporters/inmemory/InMemorySpanExporter.java	`95.45% <66.66%> (ø)`	`7.00 <0.00> (ø)`
...telemetry/exporters/zipkin/ZipkinSpanExporter.java	`87.30% <66.66%> (ø)`	`34.00 <0.00> (ø)`
...metry/exporters/logging/LoggingMetricExporter.java	`87.50% <75.00%> (ø)`	`7.00 <2.00> (ø)`
...lemetry/exporters/logging/LoggingSpanExporter.java	`86.66% <75.00%> (ø)`	`7.00 <2.00> (ø)`
...telemetry/sdk/trace/export/BatchSpanProcessor.java	`94.63% <80.00%> (-1.32%)`	`8.00 <0.00> (ø)`
...telemetry/exporters/otlp/OtlpGrpcSpanExporter.java	`77.21% <87.50%> (+1.53%)`	`5.00 <0.00> (ø)`
...metry/sdk/metrics/export/IntervalMetricReader.java	`87.69% <88.23%> (-1.40%)`	`4.00 <0.00> (ø)`
...lemetry/exporters/otlp/OtlpGrpcMetricExporter.java	`56.75% <93.75%> (+3.13%)`	`6.00 <1.00> (ø)`
...metry/sdk/common/export/CompletableResultCode.java	`93.75% <93.75%> (ø)`	`13.00 <13.00> (?)`
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 857b004...ca10135. Read the comment docs.

jkwatson · 2020-07-17T14:59:29Z

I honestly don't see how this PR improves anything for anyone. This makes a breaking change to the API for exporters, but with no actual improvement to any functionality. And, the introduction of a guava dependency on a number of exporters seems like a very large downside.

Can you explain what your end-goal is with making this change? Just adding "async"-ness for the sake of adding it doesn't seem like a change that has any value for any actual exporter author or end user.

huntc · 2020-07-17T21:42:29Z

@jkwatson We had a detailed discussion for this PR’s motivation in #1422. I asked you if you wanted me to raise a PR.

In summary, this PR will assist exporters from blocking a thread in order to return a status. The PR introduces an API change now, while we can still get away with API changes. There is no implementation benefit right now, but the opportunity is there in the future.

The Go and .NET APIs also avoid blocking.

jkwatson · 2020-07-17T21:50:22Z

I guess I had expected there to be some consumption of the Future/Callback in the SDK itself, so that there might actually be a benefit to this change. As things are today, nothing stops an exporter from doing all the work async and returning one of the enum values immediately.

So, I guess my question still stands: how do you see the SDK actually using this future? The Future API as it is also doesn't stop an exporter from making the whole call block and just returning an Future at the end of the blocking process. In fact, I believe that's what you implemented with this PR.

If we're going to make this change, let's make it meaningful, and not just pro-forma. Does that make sense?

huntc · 2020-07-17T22:13:27Z

By returning a reply for export, whether async or not, the SDK can control whether to release memory for metric and span data.

If the export reply remains synchronous then an async exporter must also decide whether to release any buffered data as a result of network failure.

Do we want this memory management logic in both places?

jkwatson · 2020-07-17T22:51:32Z

I get all that. What I don't want, though, is to have this PR land, and then not have the next steps implemented, leaving us with a half-implemented idea. Would you be willing to put together a PR that demonstrates all of this, with changes in the SDK that take advantage of this change? If we like how it all looks, we can circle back to this PR and get it merged as the first step in that larger change.

Also, if you're interested in taking on this scope of work, it would be great to have you join our weekly SIG meeting where we can talk through these plans in real time.

Sound good?

huntc · 2020-07-18T00:33:56Z

Unfortunately, having spent a number of hours already, and due to other work commitments, I’m unable to justify any advancement of this PR.

Feel free to close this PR and it’s associated issue.

jkwatson · 2020-07-18T01:47:22Z

Hmm. I'm sorry to hear that. I would really like to get working on modernizing the codebase (as much as we can given the java 7 requirement). I think these ideas will mesh well with other work-in-progress like #1419 and #1418 . We don't have a good story on export error reporting at the moment, leaving it up to the exporter authors to handle that on their own.

I think that I'd personally prefer a callback, given the cruftiness of the Future API (as @anuraaga pointed out). I think that would end up working very well in the context of #1419, and would tie us less to the antiquated Future APIs.

huntc · 2020-07-18T02:05:37Z

I don’t see callbacks as being contemporary.

If you like, I can spend some time over the weekend to look at dropping the Guava dependency and convert the export status enum to represent an async value. This should be straightforward.

If there’s no appetite for that, I understand. Sorry that I can’t spend more time than this right now.

jkwatson · 2020-07-18T02:40:18Z

I'm not sure what you mean by "convert the export status enum to represent an async value. " Can you elaborate?

I don't think callbacks are up-to-date, or contemporary, but they don't tie the APIs to what is definitely an antiquated API. At least with callbacks, we completely control the API, soup to nuts.

@anuraaga What do you think?

huntc · 2020-07-18T03:36:59Z

Perhaps you guys can save me some time by sketching out the interfaces you’d like to see.

Personally, I still see Future as being correct given that it describes the behaviour we are wanting, it isn’t deprecated and it forms the interface of the Java 8 CompletableFuture. Runtime checks can be made for handling specific types of Future.

Btw I would like to learn more about why Java 8 isn’t the baseline. It’s causing a bit of a headache here. Have you got a ref to the decision making around this?

anuraaga · 2020-07-18T03:48:22Z

@jkwatson I'm personally OK with either a callback or a custom implementation of Future that can have callbacks added. Callback seems a bit simpler to implement and I generally wouldn't avoid it out of principle since pre-Java 8 it was very common for asynchronous programming.

public interface AsyncCallback<T> {
  void onComplete(T result);
  void onError(Throwable t);
}

public void export(Collection<SpanData> spans) {
  export(spans, AsyncCallback.NO_OP);
}

public void export(Collection<SpanData> spans, AsyncCallback<ResultCode> callback);

or

public io.opentelemetry.api.Future export(Collection<SpanData> spans);

@huntc Java 7 is mostly for Android where it's still common to target older OSes for device compatibility. Not sure about this SDK repo, but the instrumentation repo has some discussion here open-telemetry/opentelemetry-java-instrumentation#599

huntc · 2020-07-18T05:12:00Z

Thanks. So, given the decision to drop support for Java 7 let’s sit on this PR until we can use CompletableFuture. Sound good?

I’m also assuming that they’ll be no GM release given the decision to migrate from Java 7.

anuraaga · 2020-07-18T05:13:34Z

@huntc The decision is to drop support for Java 8 in most instrumentation, but not ones that are common in Android. But the SDK will have to stick to Java 7 regardless, or that would effectively require all instrumentation to be Java 8.

huntc · 2020-07-18T08:11:24Z

Hi @jkwatson and @anuraaga, I've been playing around with an approach that should address all of the concerns to-date:

No Guava as a dependency
No use of Future
Leverages existing schedulers to process async results

So far, I've just coded-up a CompletableResultCode class and modified the MetricExporter calling site, IntervalMetricReader. I shall go further if we attain a consensus.

Here's the CompleteableResultCode:

/**
 * The implementation of Export operations are often asynchronous in nature, hence the need to
 * convey a result at a later time. CompletableResultCode facilitates this.
 *
 * <p>This class models JDK 8's CompletableFuture to afford migration should Open Telemetry's SDK
 * select JDK 8 or greater as a baseline, and also to offer familiarity to developers.
 */
public class CompletableResultCode {
  public CompletableResultCode() {}

  private volatile boolean succeeded = false;
  private boolean completed = false;
  private Runnable action = null;
  private Executor actionExecutor = null;

  /** The export operation finished successfully. */
  public synchronized void success() {
    if (!completed) {
      if (!succeeded) {
        succeeded = true;
      }
      if (action != null) {
        actionExecutor.execute(action);
      }
      completed = true;
    }
  }

  /** The export operation finished with an error. */
  public synchronized void failure() {
    if (!completed) {
      if (action != null) {
        actionExecutor.execute(action);
      }
      completed = true;
    }
  }

  /**
   * Obtain the current state of completion. Generally call once completion is achieved via the
   * thenRun method.
   *
   * @return the current state of completion
   */
  public boolean isSuccess() {
    return succeeded;
  }

  /**
   * Perform an action on completion. Actions are guaranteed to be called only once.
   *
   * <p>There should only be one action for this class instance.
   *
   * @param action the action to perform
   * @param executor the executor to run the action on
   */
  public synchronized void thenRunAsync(Runnable action, Executor executor) {
    if (this.action == null) {
      if (completed) {
        executor.execute(action);
      }
      this.action = action;
      this.actionExecutor = executor;
    }
  }
}

... which is then used by MetricExporter:

/**
 * Exports the collection of given {@link MetricData}.
 *
 * @param metrics the collection of {@link MetricData} to be exported.
 * @return the result of the export, which is often an asynchronous operation.
 * @since 0.1.0
 */
CompletableResultCode export(Collection<MetricData> metrics);

/**
 * Exports the collection of {@link MetricData} that have not yet been exported.
 *
 * @return the result of the flush, which is often an asynchronous operation.
 * @since 0.4.0
 */
CompletableResultCode flush();

The MetricExporter is called by the IntervalMetricReader, which ensures that no more exports occur until the previous export has finished and also avoids blocking:

final CompletableResultCode result =
    internalState.getMetricExporter().export(Collections.unmodifiableList(metricsList));
result.thenRunAsync(
    new Runnable() {
      @Override
      public void run() {
        if (!result.isSuccess()) {
          logger.log(Level.WARNING, "Metric Exporter failed");
        }
      }
    },
    exportExecutor);

My approach bears similarity to Java 8's CompletableFuture for the sake of developer familiarity and the potential of future migration.

Thoughts?

carlosalberto · 2020-08-03T00:38:39Z

Hey @huntc

Is there a consensus required between the maintainers?

Yes (at least we usually try to go that way :) )

Exporters perform long-lived operations. Their method signatures should reflect that out of correctness.

I see the Disruptor processor has been deemed as a thing to avoid (for some cases, I'm suspecting), but what about using BatchSpanProcessor? What's the reason to avoid using that one? I'm still wondering what case the current design fails to address.

On the .Net/Go mention, I'd definitely put Go out of the comparison as it's a different animal IMHO. But I'm curious about the .Net take on this and will check it out.

Also, I'd love to hear @bogdandrutu's opinion, as he was the one originally designing the processor/exporter layer, and he might have something to say.

EDIT: Trying to go over the different comments, it seems mostly cancelation is why this is needed? Would love to see also a code example that makes use of the new proposed API.

huntc · 2020-08-03T00:47:38Z

... but what about using BatchSpanProcessor? What's the reason to avoid using that one? I'm still wondering what case the current design fails to address.

I'm a fan of the simple span processor because there's no thread pool other than any required by the exporter(s). No thread pool == more efficient memory usage. Threads are expensive. The downside of the simple span processor is that if an exporter blocks, so will my program. Exporters should thus never block their calling threads.

Again, getting the method signature right for the exporter is my primary concern. The current design fails to convey that exporter operations are long-lived. Types are important.

carlosalberto · 2020-08-03T00:55:05Z

I'm a fan of the simple span processor because there's no thread pool other than any required by the exporter(s). No thread pool == more efficient memory usage. Threads are expensive.

Oh, that's very interesting. Sounds reasonable, yes, and worth trying out.

The current design fails to convey that exporter operations are long-lived.

True, but I'd also love to see this change validated with some actual prototype that takes full advantage.

(Hope @bogdandrutu can chime in real soon ;) )

huntc · 2020-08-03T01:02:48Z

True, but I'd also love to see this change validated with some actual prototype that takes full advantage.

I'm unsure what you mean by "advantage". It's more a question of correctness. If you're looking for an async exporter prototype though then perhaps the following test will be illustrative:

opentelemetry-java/sdk/src/test/java/io/opentelemetry/sdk/trace/export/BatchSpanProcessorTest.java

Line 314 in b7cfcab

@Test

Note that prior to this PR, canceling exports via interrupting them is unsafe. So, perhaps that's another good reason then. :-)

But again, my motivation here is mostly around the subject of correctness.

exporters/otlp/src/main/java/io/opentelemetry/exporters/otlp/OtlpGrpcMetricExporter.java

anuraaga · 2020-08-03T01:22:52Z

sdk/src/main/java/io/opentelemetry/sdk/trace/export/MultiSpanExporter.java

-    ResultCode currentResultCode = SUCCESS;
+  public CompletableResultCode flush() {
+    final CompletableResultCode compositeResultCode = new CompletableResultCode();
+    final CountDownLatch completionsToProcess = new CountDownLatch(spanExporters.length);


If not waiting on it, may as well use AtomicInteger to clarify that it's just for counting.

anuraaga · 2020-08-03T01:24:57Z

sdk/src/main/java/io/opentelemetry/sdk/trace/export/SimpleSpanProcessor.java

-      spanExporter.export(spans);
-    } catch (Throwable e) {
-      logger.log(Level.WARNING, "Exception thrown by the export.", e);
+    if (exportAvailable.compareAndSet(true, false)) {


For SimpleSpanProcessor I think I'd expect it to be simple and just call export in all cases (if there is a backup in the exporter, that is a reason to use a more complicated span processor like batch span processor).

That would then require exporters to become re-entrant. My assumption at present, as per the existing blocking API, is that only one export is called at a time (which is probably something we should document - it can simplify the authoring of exporters greatly - wdyt?).

I'm not sure what you mean by "re-entrant" here. Do you just mean having more than one export running at a time, or are you actuallyreferring to sending the same spans more than once and having the exporter be able to handle de-duping them?

Meanwhile, I've added some API docs to clearly state the position on re-entrancy.

I'm not sure what you mean by "re-entrant" here. Do you just mean having more than one export running at a time, or are you actuallyreferring to sending the same spans more than once and having the exporter be able to handle de-duping them?

I was referring to the number of export or flush calls in flight at one time, which is what this code is associated with. We only want one of each of these calls in flight. In fact, we shouldn't really call export and flush at the same time either. Keeps the programming model simpler for exporters.

Having said that, the IntervalMetricReader has no such back-pressure mechanism, and never did.

So, should I re-instate the ability to only call export one at a time for the IntervalMetricReader and BatchSpanProcessor, and have them drop any new telemetry in the case where an export is in-flight? We can leave the SimpleSpanProcessor as-is though i.e. without the "in-flight" check. The SimpleSpanProcessor will be perfect for those more sophisticated exporters, such as what can be written with Akka-streams.

I've now pushed a commit with updated documentation for the exporters, and the reinstatement of backpressure handling for the IntervalMetricReader and BatchSpanProcessor.

BTW Is there an equivalent of SimpleSpanProcessor for metrics?

Metrics have a somewhat different pipeline structure. The closest equivalent is probably the IntervalMetricReader class for push-based exporters. Pull-based, of course, manage all of that themselves.

huntc · 2020-08-04T07:49:30Z

(Hope @bogdandrutu can chime in real soon ;) )

FYI All that remains is for me to write a test for the CompletableResultCode, then this PR should be done. I hope that @anuraaga 's contribution here with the async OTLP exporter is also illuminating.

exporters/otlp/src/main/java/io/opentelemetry/exporters/otlp/OtlpGrpcMetricExporter.java

huntc · 2020-08-07T00:51:00Z

Please don't work (much) on this more until we have reached an agreement - I know now it's all excitement, but I'm very wary of sexy changes that later we may need to discard 😞

@carlosalberto Can we please have some resolution on this? I'll happily complete the PR over the weekend, but not if it isn't destined for greatness. :-)

jkwatson · 2020-08-07T03:12:23Z

I'll put this on the agenda for tomorrow's SIG meeting, so it doesn't get lost.

huntc · 2020-08-08T05:25:25Z

I'll put this on the agenda for tomorrow's SIG meeting, so it doesn't get lost.

Thanks. Any resolution?

jkwatson · 2020-08-10T17:11:37Z

I'll put this on the agenda for tomorrow's SIG meeting, so it doesn't get lost.

Thanks. Any resolution?

The decision (which is not a resolution, unfortunately) is that Carlos is going to find some time for a deeper dive into this PR. Meanwhile, the discussion on the original issue (#1422) has some further thoughts from other folks (@Oberon00 in particular)

anuraaga · 2020-08-11T01:32:23Z

@carlosalberto Is it possible to get a commitment to that deep dive over the next few days? I understand you're busy, but this has had many weeks for a closer look and I don't think it's fair to keep it stuck.

carlosalberto · 2020-08-14T15:28:19Z

Let's go with it. 👍

jkwatson · 2020-08-14T16:22:37Z

@huntc We're a go for this. Can you get it rebased? Thanks so much for your patience. You work is appreciated, and sorry that it took a long time to get this landed.

This commit recognises that the export and flush methods of span and trace exporters can be, and often are, implemented with long-lived operations over networks. We therefore see that these method return types are represented using Java's Future type to account for this common behaviour.

anuraaga

Thanks a lot for the patience! Just nits but LGTM

anuraaga · 2020-08-15T02:22:55Z

exporters/logging/src/main/java/io/opentelemetry/exporters/logging/LoggingMetricExporter.java

    for (Handler handler : logger.getHandlers()) {
      try {
        handler.flush();
      } catch (Throwable t) {
-        resultCode = ResultCode.FAILURE;
+        resultCode.fail();


Should we early return here? Similar comment elsewhere

I think it doesn't matter in this particular case, since we might want to flush all the handlers, even if one fails. The call to succeed() below is a no-op if it's been previously failed.

I'll add a return to make it clear but, as John mentioned, it makes no diff.

anuraaga · 2020-08-15T02:27:03Z

sdk/src/main/java/io/opentelemetry/sdk/common/export/CompletableResultCode.java

+import javax.annotation.concurrent.GuardedBy;
+
+/**
+ * The implementation of Export operations are often asynchronous in nature, hence the need to


This could be second sentence, first sentences should describe the class. e.g. A result which will be completed at a later time asynchronously.

Sentences swapped, although my personal preference is to lead with the rationale as the type name is good enough.

anuraaga · 2020-08-15T02:28:29Z

sdk/src/main/java/io/opentelemetry/sdk/common/export/CompletableResultCode.java

+ * select JDK 8 or greater as a baseline, and also to offer familiarity to developers.
+ */
+public class CompletableResultCode {
+  /** A convenience for declaring success. */


Returns a {@link CompletableResultCode} that has been completed successfully.

Ditto for below with unsuccessfully

Realized we can probably just copy paste javadoc from CompletableFuture

anuraaga · 2020-08-15T02:29:26Z

sdk/src/main/java/io/opentelemetry/sdk/common/export/CompletableResultCode.java

+
+  private final Object lock = new Object();
+
+  /** The export operation finished successfully. */


Completed this {@link CompletableResultCode} successfully.

jkwatson · 2020-08-17T17:32:48Z

sdk/src/main/java/io/opentelemetry/sdk/trace/export/SimpleSpanProcessor.java

@@ -71,15 +74,25 @@ public boolean isStartRequired() {
  }

  @Override
+  @SuppressWarnings("BooleanParameter")


I don't think this is needed on this method.

Hangover from the past. Removed.

jkwatson · 2020-08-17T17:40:31Z

sdk/src/main/java/io/opentelemetry/sdk/common/export/CompletableResultCode.java

+  /** The export operation finished successfully. */
+  public CompletableResultCode succeed() {
+    synchronized (lock) {
+      if (succeeded == null) {


This behavior should be documented, since I think it's non-obvious that it's first-in-wins with succeed/fail.

Also, unit tests to verify it would also be very good.

Done. New test added also.

jkwatson · 2020-08-17T17:42:53Z

sdk/src/main/java/io/opentelemetry/sdk/common/export/CompletableResultCode.java

+  private Boolean succeeded = null;
+
+  @GuardedBy("lock")
+  private final ArrayList<Runnable> completionActions = new ArrayList<>();


This can be declared as a simple List on the LHS, correct?

jkwatson · 2020-08-17T17:59:14Z

I have just a couple of minor issues, mostly just that we should document the first-in-wins behavior of the result code, and write unit tests to verify that behavior.

huntc · 2020-08-18T05:30:34Z

I have just a couple of minor issues, mostly just that we should document the first-in-wins behavior of the result code, and write unit tests to verify that behavior.

PR feedback addressed and is in as a separate commit.

jkwatson

Thanks again! Let's

huntc requested review from arminru, bogdandrutu, carlosalberto, jkwatson, pavolloffay, thisthat and tylerbenson as code owners July 17, 2020 04:35

huntc mentioned this pull request Jul 17, 2020

Should exports be async #1422

Closed

huntc commented Jul 17, 2020

View reviewed changes

exporters/inmemory/build.gradle Outdated Show resolved Hide resolved

anuraaga reviewed Jul 17, 2020

View reviewed changes

...rters/inmemory/src/main/java/io/opentelemetry/exporters/inmemory/InMemoryMetricExporter.java Outdated Show resolved Hide resolved

huntc commented Jul 17, 2020

View reviewed changes

...s/inmemory/src/test/java/io/opentelemetry/exporters/inmemory/InMemoryMetricExporterTest.java Outdated Show resolved Hide resolved

huntc commented Jul 17, 2020

View reviewed changes

exporters/logging/src/main/java/io/opentelemetry/exporters/logging/LoggingMetricExporter.java Outdated Show resolved Hide resolved

huntc commented Jul 17, 2020

View reviewed changes

exporters/logging/src/main/java/io/opentelemetry/exporters/logging/LoggingSpanExporter.java Outdated Show resolved Hide resolved

huntc force-pushed the export-futures branch from 79097e7 to 3e662fd Compare July 17, 2020 05:01

huntc force-pushed the export-futures branch from f67c7b3 to b7cfcab Compare August 3, 2020 00:10

anuraaga reviewed Aug 3, 2020

View reviewed changes

jkwatson reviewed Aug 4, 2020

View reviewed changes

exporters/otlp/src/main/java/io/opentelemetry/exporters/otlp/OtlpGrpcMetricExporter.java Show resolved Hide resolved

huntc force-pushed the export-futures branch from 6ed3e68 to dfc71f6 Compare August 14, 2020 22:28

huntc force-pushed the export-futures branch from dfc71f6 to a1c86fa Compare August 14, 2020 22:53

huntc marked this pull request as ready for review August 14, 2020 22:56

anuraaga approved these changes Aug 15, 2020

View reviewed changes

jkwatson reviewed Aug 17, 2020

View reviewed changes

PR feedback

ca10135

jkwatson approved these changes Aug 18, 2020

View reviewed changes

jkwatson merged commit d6b53e6 into open-telemetry:master Aug 18, 2020

huntc deleted the export-futures branch August 18, 2020 19:28

huntc mentioned this pull request Aug 20, 2020

Leverage BatchSpanProcessor's span dropping functionality #1566

Closed


		private final Object lock = new Object();

		/** The export operation finished successfully. */

Makes Export methods async #1428

Makes Export methods async #1428

Conversation

huntc commented Jul 17, 2020 • edited Loading

linux-foundation-easycla bot commented Jul 17, 2020 • edited Loading

codecov bot commented Jul 17, 2020 • edited Loading

Codecov Report

jkwatson commented Jul 17, 2020

huntc commented Jul 17, 2020

jkwatson commented Jul 17, 2020

huntc commented Jul 17, 2020 • edited Loading

jkwatson commented Jul 17, 2020

huntc commented Jul 18, 2020

jkwatson commented Jul 18, 2020

huntc commented Jul 18, 2020 • edited Loading

jkwatson commented Jul 18, 2020

huntc commented Jul 18, 2020

anuraaga commented Jul 18, 2020

huntc commented Jul 18, 2020 • edited Loading

anuraaga commented Jul 18, 2020

huntc commented Jul 18, 2020 • edited Loading

carlosalberto commented Aug 3, 2020 • edited Loading

huntc commented Aug 3, 2020 • edited Loading

carlosalberto commented Aug 3, 2020

huntc commented Aug 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huntc Aug 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huntc Aug 3, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huntc commented Aug 4, 2020

huntc commented Aug 7, 2020

jkwatson commented Aug 7, 2020

huntc commented Aug 8, 2020

jkwatson commented Aug 10, 2020

anuraaga commented Aug 11, 2020

carlosalberto commented Aug 14, 2020

jkwatson commented Aug 14, 2020

anuraaga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkwatson commented Aug 17, 2020

huntc commented Aug 18, 2020

jkwatson left a comment

Choose a reason for hiding this comment

huntc commented Jul 17, 2020 •

edited

Loading

linux-foundation-easycla bot commented Jul 17, 2020 •

edited

Loading

codecov bot commented Jul 17, 2020 •

edited

Loading

huntc commented Jul 17, 2020 •

edited

Loading

huntc commented Jul 18, 2020 •

edited

Loading

huntc commented Jul 18, 2020 •

edited

Loading

huntc commented Jul 18, 2020 •

edited

Loading

carlosalberto commented Aug 3, 2020 •

edited

Loading

huntc commented Aug 3, 2020 •

edited

Loading

huntc commented Aug 3, 2020 •

edited

Loading

huntc Aug 3, 2020 •

edited

Loading

huntc Aug 3, 2020 •

edited

Loading