Move stackdriver trace exporter to new interface and pdata

[stevencl1013]

Description: Changes stackdriver trace exporter to use pdata and implement the new exporter interface rather than the old, and use the OT Go stackdriver exporter rather than the OC one. The metric exporter is just converting from pdata back to consumerdata and still using the OC exporter.

Testing: Modified the tests for stackdriver, data conversion, and factory to reflect changes.

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Steven Lee (120c4ff, 32b8f93, 4811519, 1d1a071, 0759ada, 63413c4, 75698ea, 926ac33, 3a02d94, 6d9d8d8, 5432bae, db1db6e, 16ea74f, efe20d3, 619c749, fd3178a, 2423032, 1285b8d, 2d4ed44, 41910d9, 6d681b0, 7f94ea5, b4f771c)

[nilebox]

@stevencl1013 I think I've found the root cause after debugging the test:

2020/07/27 16:11:20 Failed to export to Stackdriver: rpc error: code = Canceled desc = context canceled
    TestStackdriverExport: stackdriver_test.go:105: test timed out

Note the first error message. It comes from cloudtrace.go#L221 (SDK exporter you have imported in this PR).

So we fail to export, because the context has been cancelled. The question is what is that context and why has it been cancelled early? The answer is related to open-telemetry/opentelemetry-collector#1386 as you suspected.

• timeoutSender creates a new context per request, and cancels it in common.go#L229, right after export function completes in common.go#L231
• SDK CloudTrace exporter is using bundler, which exports data asynchronously, as you've implemented it in Add bundler to exporter GoogleCloudPlatform/opentelemetry-operations-go#39
• This means that CloudTrace request is invoked in trace.go#L143 after the context has been cancelled as I described above

Hopefully it's not too confusing?

The workaround is trivial: don't forward the context passed into collector exporter to SDK exporter, and create a new context instead.
In other words, in the pushTraces function do the following:

        // NOTE: SDK Trace exporter exports data asynchronously via bundler, 
        // so we can't use the parent (synchronous) context, which gets cancelled 
        // immediately after this function is executed.
        ctx = context.Background() // create a new context to avoid cancellation
	for _, span := range spans {
		te.texporter.ExportSpan(ctx, span)
		goodSpans++
	}

Alternatively, we may make the use of bundler configurable, i.e. support "synchronous mode" which would've made more sense here.

/cc @james-bebbington

[james-bebbington]

The workaround is trivial: don't forward the context passed into collector exporter to SDK exporter, and create a new context instead.

Great work finding this issue.

I think it would make more sense to put the fix into operations-go instead though, as the same issue could occur for other clients of that library with its current implementation. To avoid this kind of issue entirely, we should ensure we always create a new context at the time we start asynchronous work.

i.e. do not pass context into bundler; instead you can extract the spans from context first, and pass those in or do something similar.

[codecov]

Codecov Report

Merging #486 into master will increase coverage by 0.03%.
The diff coverage is 68.20%.

@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
+ Coverage   86.26%   86.30%   +0.03%     
==========================================
  Files         195      195              
  Lines       10632    10579      -53     
==========================================
- Hits         9172     9130      -42     
+ Misses       1128     1123       -5     
+ Partials      332      326       -6     

Flag	Coverage Δ
#integration	71.09% <ø> (ø)
#unit	86.13% <68.20%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
exporter/stackdriverexporter/stackdriver.go	35.55% <40.00%> (-8.39%)	⬇️
exporter/stackdriverexporter/factory.go	68.42% <80.00%> (+25.56%)	⬆️
exporter/stackdriverexporter/spandata.go	82.90% <84.34%> (+12.62%)	⬆️
exporter/awsxrayexporter/translator/cause.go	94.73% <0.00%> (-5.27%)	⬇️

---

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 32c9e6a...ca94625. Read the comment docs.

[stevencl1013]

@nilebox Thanks for finding the cause of the issue! I committed the change and it seems to be working now, but I'll also look into modifying the operations-go repo as @james-bebbington mentioned.

[nilebox]

Looks good, just need a couple of final cleanups.

[nilebox]

@stevencl1013 the build is failing, please ping me when it's ready for another round of reviews.

[nilebox]

LGTM, thanks for patiently addressing issues and feedback :)

[nilebox]

@bogdandrutu Just noticed that my approval is not "green", could you add me to approvers in the contrib repo as well?