OpenTelemetry Collector Demo

[stefanprisca]

Example of exporting traces from the go-sdk to the OpenTelemetry Collector, and from there to Jaeger. This PR addresses issue #643 .

[stefanprisca]

I am not sure why the build is failing, as the test-386 is passing locally.

[MrAlias]

Overall looks good with minor formatting suggestions. The important thing to change though is the Go code suggestions. I think we want the example to be verbose and explicit as a basis to develop from.

I think we could just fully commit to running all the dependencies in Kubernetes and include Deployments for both the collector and Jaeger in this example. This could also be included in a follow up if you think that is better.

[stefanprisca]

I think we could just fully commit to running all the dependencies in Kubernetes and include Deployments for both the collector and Jaeger in this example. This could also be included in a follow up if you think that is better.

Agreed, but I wasn't sure how much of the other dependencies you would want as part of this example, so I kept it to a minimum :) I will add them as part of this commit as there shouldn't be too many more files (unless you want to merge this first to close the issue).

[MrAlias]

I think we could just fully commit to running all the dependencies in Kubernetes and include Deployments for both the collector and Jaeger in this example. This could also be included in a follow up if you think that is better.

Agreed, but I wasn't sure how much of the other dependencies you would want as part of this example, so I kept it to a minimum :) I will add them as part of this commit as there shouldn't be too many more files (unless you want to merge this first to close the issue).

I'm in favor of adding to this PR. 👍

[MrAlias]

Looks like go.sum needs to be synced. You can do this with make precommit or directly with go mod tidy.

[stefanprisca]

I think this is pretty much ready :) I updated to include both the Jaeger and OpenTelemetry Collector kubernetes deployments, but kept the demo application outside the cluster. I think it's ok this way, as the example can be used to get started with a functional connection and illustrates the code behind it. Then it's simple enough to build a docker image and deploy the application inside the cluster. Also, I used a NodePort to connect the application to the Collector running inside k8s, as it's an easy way to get a static port/ip for the example, and it works on both local and hosted clusters. It's also easy to change it afterwards to a ClusterIP or LoadBalancer depending on the use case.

[lizthegrey]

paging @puckpuck and @irvingpop to help me review this

[MrAlias]

Looks awesome!

Missing license headers for the Kubernetes manifests, otherwise 🚀

[puckpuck]

I deployed this in my test environment, and though the spans did get through to Jaeger, I noticed some oddities about it that were not apparent until i really dug in. The initial and final span seem to never make it into Jaeger.

In my setup, I ran the code locally inside of VS code, pointed to an opentelemetry-collector deployed in K8s exposed via a loadbalancer. The K8s environment is on another host within my local network.

To solve this, I removed the defer span.End() for the initial span, ended it after the final loop, and injected a 1 second delay to ensure the otlp exporter can finish sending.

	// Then use the OpenTelemetry tracing library, like we normally would.
	ctx, span := tracer.Start(context.Background(), "CollectorExporter-Example")

	for i := 0; i < 10; i++ {
		_, iSpan := tracer.Start(ctx, fmt.Sprintf("Sample-%d", i))
		<-time.After(time.Second)
		iSpan.End()
	}

	span.End()

        // Wait 1 second before ending
	<-time.After(time.Second)

[stefanprisca]

True, I missed that.
But I would expect that when ending a span it is pushed by default. It seems a bit weird to have to specifically wait.
Also, when using the otlp exporter as Syncer it pushes the traces without having to wait:

	tp, err := sdktrace.NewProvider(
...
		sdktrace.WithSyncer(exp))

I updated the PR with the suggested change, but I think this should be an issue on the batcher side. What do you guys think?

[puckpuck]

ahhh... using sdktrace.WithSyncer(exp) instead of batch makes sense for this example.

[MrAlias]

I think this should be an issue on the batcher side.

I think this is something actively being discussed in other threads.

using sdktrace.WithSyncer(exp) instead of batch makes sense for this example.

I agree with this. Let's merge this and switch to a syncer in a subsequent PR.