Skip to content
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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

flands
Copy link

@flands 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
@arey
Copy link
Member

arey commented May 9, 2020

Hi @flands Could tou explain us what OpenTelemtry brings to Petclinic?
The readme.md has not be updated. And how collector.yamlis working?

@flands
Copy link
Author

flands commented May 18, 2020

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.

@flands
Copy link
Author

flands commented May 18, 2020

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.

Copy link
Member

@arey arey left a 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"
Copy link
Member

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

@@ -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
Copy link
Member

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.

Copy link
Author

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.

@@ -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"]
Copy link
Member

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?

Copy link
Author

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.

@marcingrzejszczak
Copy link
Contributor

Why not just use Spring Cloud Sleuth OTel (https://github.com/spring-cloud-incubator/spring-cloud-sleuth-otel/) ?

@marcingrzejszczak
Copy link
Contributor

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

@arey
Copy link
Member

arey commented Apr 11, 2021

I didn't know the Spring Cloud Sleuth OTel project. Sure we could use it. I note that it is part of an incubator and hope that that it could join the Spring Could project.
@flands do you agree with the proposal of@marcingrzejszczak?

@marcingrzejszczak
Copy link
Contributor

Yeah once OTel is GA without the alpha things in it we will consider merging it into Sleuth.

@pranta
Copy link

pranta commented Sep 29, 2021

@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!

Copy link

sonarcloud bot commented Jan 5, 2024

Quality Gate Passed Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

4 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants