Implemented the Jaeger Span Exporter using gRPC as the transport

[jpkrohling]

Closes #469 by implementing a Jaeger Span Exporter

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

[jpkrohling]

Marking this as WIP while I run some manual tests against an actual Jaeger collector.

[jpkrohling]

This is how a span looks like on Jaeger, with a code similar to this:

    ManagedChannel channel = ManagedChannelBuilder.forAddress("localhost", 14250).usePlaintext().build();
    JaegerGrpcSpanExporter exporter = JaegerGrpcSpanExporter.newBuilder()
            .setChannel(channel)
            .setServiceName("instrumented-with-opentelemetry")
            .setDeadline(0)
            .build();

    TracerSdk tracer = new TracerSdk();
    tracer.addSpanProcessor(SimpleSampledSpansProcessor.newBuilder(exporter).build());

    io.opentelemetry.trace.Span span = tracer.spanBuilder("my-operation").startSpan();
    span.end();

[bogdandrutu]

Initial set of comments.

[codecov-io]

Codecov Report

Merging #481 into master will increase coverage by 0.2%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #481     +/-   ##
===========================================
+ Coverage     77.96%   78.17%   +0.2%     
- Complexity      555      556      +1     
===========================================
  Files            67       67             
  Lines          1970     1970             
  Branches        186      186             
===========================================
+ Hits           1536     1540      +4     
+ Misses          372      367      -5     
- Partials         62       63      +1

Impacted Files	Coverage Δ	Complexity Δ
...ntelemetry/sdk/trace/RecordEventsReadableSpan.java	98.79% <0%> (+2.4%)	55% <0%> (+1%)	⬆️

---

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 fc74e77...64bea08. Read the comment docs.

[jpkrohling]

PR updated. I set the items I fixed as "marked as resolved", but there are a couple of items still open. The most important is about looking into the status code from the collector and determining whether the call should be retried.

[bogdandrutu]

Please rebase.

[bogdandrutu]

@pavolloffay do you think going with submodules is the right thing to do? If so where are these protos?

[pavolloffay]

I think there are two viable solutions

• include protos as a GIT submodule from https://github.com/jaegertracing/jaeger-idl and build the java classes here
• use the same approach as we use for OpenTelemetry proto classes. Build the Jaeger proto outside of this project and consume just as a dependency. Possibly build them in a new lightweight submodule in https://github.com/jaegertracing/jaeger-client-java

If we build them here we control the protobuf compiler versions. However I am not sure if that is an issue or not, but otherwise maybe compiling them outside of this repo seems better to me.

[yurishkuro]

Submodule won't work just yet as we don't publish Jaeger proto to jaeger-idl (because of gogo annotations, jaegertracing/jaeger-idl#55).

I do not recommend building classes in an outside repo either, because it creates a potential conflict of protobuf-related library versions. The IDLs are the official interface for interoperability.

What is the concern of simply copying proto files? It's not much different from a submodule approach, you still need to manually pick newer versions.

[bogdandrutu]

To not block this PR, I would suggest that we keep the copy for the moment of the proto, but I would suggest that we document in a README file in this modules where the protos are copied

[jpkrohling]

To not block this PR, I would suggest that we keep the copy for the moment of the proto, but I would suggest that we document in a README file in this modules where the protos are copied

Done.

[pavolloffay]

This looks good! I had some minor comments.

How do we feel about adding e2e tests? This could be easily done using docker via java-testcontainers https://github.com/testcontainers/testcontainers-java with jaeger all-in-one distribution. Here are some examples

• https://github.com/opentracing-contrib/java-spring-jaeger/blob/master/opentracing-spring-jaeger-web-starter-it/src/test/java/io/opentracing/contrib/java/spring/web/jaeger/starter/it/JaegerIntegrationTest.java#L49
• https://github.com/jaegertracing/spark-dependencies/blob/master/jaeger-spark-dependencies-elasticsearch/src/test/java/io/jaegertracing/spark/dependencies/elastic/JaegerElasticsearchEnvironment.java#L63

[pavolloffay]

Should be SDK as provided dependency?

[jpkrohling]

Did you mean implementation instead of provided? I think it's appropriate to have api here, as methods from this module accept and return instances of classes defined by the SDK module.

An API dependency is one that contains at least one type that is exposed in the library binary interface, often referred to as its ABI (Application Binary Interface).

https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_recognizing_dependencies

[pavolloffay]

How is api translated to maven dependency type? Is it compile or provided? I thought it is provided.

Provided means that you need the JAR for compiling, but at run time there is already a JAR provided by the environment so you don't need it packaged with your app. For a web app, this means that the JAR file will not be placed into the WEB-INF/lib directory.

[jpkrohling]

The differences in Gradle are mostly about what is exported to consumers of the library: an application declaring a dependency on the exporters/jaeger module will have opentelemetry-sdk as a transitive dependency, as it's declared as api. The dependencies that exporters/jaeger declare as implementation aren't transitive dependencies for a consumer.

There's a runtimeOnly mode as well, which is probably close to what provided does in Maven. But still, I don't think it's the right mode. api still seems the most appropriate here.

[jpkrohling]

How do we feel about adding e2e tests?

I'm all for it, on a follow-up PR.

[pavolloffay]

@jpkrohling could you please open an issue so we can discuss it there?

[bogdandrutu]

@jpkrohling please file the issue that @pavolloffay mentioned.

[jpkrohling]

Created #498

[yarjunrao]

can you give me full example of this ..#481 (comment)
i am getting problem when trying same in my local below error throwing.
WARNING: 2019/12/30 16:22:29 grpc: Server.Serve failed to create ServerTransport: connection error: desc = "transport: http2Server.HandleStreams received bogus greeting from client: "\x16\x03\x01\x00\x82\x01\x00\x00~\x03\x03\x94\x1e\x81\xb2\xbfk\x1a~\xe3\xbb\v\u007f\xa6""
WARNING: 2019/12/30 16:22:29 grpc: Server.Serve failed to create ServerTransport: connection error: desc = "transport: http2Server.HandleStreams received bogus greeting from client: "\x16\x03\x01\x00\x82\x01\x00\x00~\x03\x03\xde\x19\x01\xec\xccՊ\xfa\xc2~~\xa9\xe9""

[thisthat]

It seems you have problem with TLS. You can check this example how to fully create a tracer which exports to Jaeger: https://github.com/dynatrace-oss-contrib/opentelemetry-java/tree/add-jaeger-example/examples/jaeger