-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add OpenTelemetry distributed tracing #149
base: main
Are you sure you want to change the base?
Conversation
flands
commented
May 8, 2020
- Add Otel Java auto-instrumentation with Jaeger export
- Add Otel Collector
- Configure data to be sent to Zipkin
* Add Otel Java auto-instrumentation with Jaeger export * Add Otel Collector * Configure data to be sent to Zipkin
Hi @flands Could tou explain us what |
Hey @arey this PR brings distributed tracing to the microservices application. I can update the README. The collector is used to forward the received trace data to Zipkin. |
Took a look at the README and it does not say much about Zipkin -- added Otel Collector. Collector was added so users could switch to a different backend if desired. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know the OpenTelemtry
project. But the OpenTracing
project yes. I just see that OpenTelemetry
is now replacing OpenTelemtry
.
I wanted to know if the Spring Cloud
team is planning to support OpenTelemetry and I found this interesting discussion: spring-cloud/spring-cloud-sleuth#1497
I'm not sure that OpenTelemetry
has right now its place to the Spring Petclinic Microservices
project. I propose to add it in a dedicated git branch that you will in charge to maintain and sync with develop. @mszarlinski what is your opinion?
collector.yaml
Outdated
logging: | ||
|
||
zipkin: | ||
url: "http://tracing-server:9411/api/v2/spans" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 9411 port should not be hardcoced. It's coming from the OpenZipkin configuration file: https://github.com/spring-petclinic/spring-petclinic-microservices-config/blob/master/tracing-server.yml
docker/Dockerfile
Outdated
@@ -6,14 +6,28 @@ ARG DOCKERIZE_VERSION | |||
RUN wget -O dockerize.tar.gz https://github.com/jwilder/dockerize/releases/download/${DOCKERIZE_VERSION}/dockerize-alpine-linux-amd64-${DOCKERIZE_VERSION}.tar.gz | |||
RUN tar xzf dockerize.tar.gz | |||
RUN chmod +x dockerize | |||
RUN wget -O opentelemetry-auto-0.2.2.jar https://github.com/open-telemetry/opentelemetry-auto-instr-java/releases/download/v0.2.2/opentelemetry-auto-0.2.2.jar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it possible to avoid wget the OpenTelemtry artifacts
and prefer Maven
depencies?
Moreover, the Open Telemetry version 0.2.2
is hardcoded at a lot of places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it appears the auto-instrumentation is not published on Maven yet -- I will file an issue for this to get it fixed.
docker-compose.yml
Outdated
@@ -14,7 +14,7 @@ services: | |||
mem_limit: 512M | |||
depends_on: | |||
- config-server | |||
entrypoint: ["./dockerize","-wait=tcp://config-server:8888","-timeout=60s","--","java", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseCGroupMemoryLimitForHeap", "-Djava.security.egd=file:/dev/./urandom","-jar","/app.jar"] | |||
entrypoint: ["./dockerize","-wait=tcp://config-server:8888","-timeout=60s","--","java", "-javaagent:/opt/opentelemetry-auto-0.2.2.jar", "-Dota.exporter.jar=/opt/opentelemetry-auto-exporters-jaeger-0.2.2.jar", "-Dota.exporter.jaeger.endpoint=otel-collector:14250", "-Dota.exporter.jaeger.service.name=discovery", "-XX:+UnlockExperimentalVMOptions", "-XX:+UseCGroupMemoryLimitForHeap", "-Djava.security.egd=file:/dev/./urandom","-jar","/app.jar"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it exists a better integration with Spring Boot / Spring Could?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is bytecode (auto) instrumentation -- for Java this is done via -javaagent
. This is a common industry practice done by many vendors. The goal is not to be Java framework specific.
Why not just use Spring Cloud Sleuth OTel (https://github.com/spring-cloud-incubator/spring-cloud-sleuth-otel/) ? |
Since Sleuth is on the classpath you can just do the following <properties>
<spring-cloud-sleuth-otel.version>1.0.0-M7</spring-cloud-sleuth-otel.version>
</properties>
<dependencies>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-starter-sleuth</artifactId>
<exclusions>
<exclusion>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-sleuth-brave</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>org.springframework.cloud</groupId>
<artifactId>spring-cloud-sleuth-otel-autoconfigure</artifactId>
</dependency> and you have OpenTelemetry support with no additional work needed |
I didn't know the |
Yeah once OTel is GA without the alpha things in it we will consider merging it into Sleuth. |
@flands : I tried cloning your changes but am running to some Mac related issues which I posted as a question on StackOverflow: https://stackoverflow.com/questions/69368828/unable-to-access-docker-container-web-page-in-spite-of-port-mapping-on-a-macos-b . Please help if you can. Cheers! |
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 4 New issues |