Skip to content
This repository has been archived by the owner on Sep 2, 2024. It is now read-only.

Add OpenTelemetry trace functionality to cartservice #744

Merged
merged 13 commits into from
Jun 15, 2021
Merged

Conversation

arbrown
Copy link
Collaborator

@arbrown arbrown commented Jun 10, 2021

Adds automatic grpc trace to cartservice
Adds sidecar deployment of OpenTelemetry collector to existing cartservice deployment
Adds new OpenTelemetry collector deployment to collect trace information from cartservice (and potentially other services in the future)

* Gateway collector collects from agents and forwards to cloud project
* Agent collector in cartservice will forward trace information to gateway
* Add OpenTelemetry agent to docker-compose.yaml
The initContainer retrieves the current Google Cloud project ID and inserts it into the Collector config file.
* Add copyright header
Explicitly specifying versions of GRPC and OTel libraries until the OTel libraries are officially released.
@arbrown arbrown requested a review from 0xSage as a code owner June 10, 2021 18:44
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Jun 10, 2021
@arbrown arbrown linked an issue Jun 10, 2021 that may be closed by this pull request
4 tasks
@simonz130 simonz130 assigned arbrown and unassigned keenan-devrel Jun 10, 2021
Copy link

@simonz130 simonz130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great progress, @arbrown! Thank you!

I left a few notes and there is also a general note:

  1. We need to update the architecture diagram for the demo app to reflect the fact that we're using a collector to capture cartservice traces
  2. We should add a readme with dive-in architecture of cart service in the root folder of cart service code. I'm familiar with collector/receiver nomenclature, but folks who are not connected to OpenTelemetry might find it hard to follow. The readme should clarify the components used to capture telemetry, their role and key configuration considerations.

Since your current PR is quite large, maybe we should do (1) and (2) as separate PRs.

kubernetes-manifests/cartservice.yaml Outdated Show resolved Hide resolved
Comment on lines 99 to 104
exporters:
otlp:
endpoint: "opentelemetrycollector.default.svc.cluster.local:4317"
insecure: true
logging:
loglevel: debug

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Sandbox is educational tool, please add extensive comments on the meaning of different fields you use for configuring collector in yaml.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified the yaml and added explanatory comments where I thought it was appropriate. Please let me know if there are other parts you think need additional explanation.

kubernetes-manifests/cartservice.yaml Outdated Show resolved Hide resolved
kubernetes-manifests/opentelemetrycollector.yaml Outdated Show resolved Hide resolved
image: "redis:alpine"
otel-agent:
image: "otel/opentelemetry-collector-contrib-dev:latest"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I pin the redis image as well? It looks like it's been on redis:alpine this whole time. Looks like we could pin it to redis:6.2-alpine3.13 which is where redis:alpine sits now.

src/cartservice/cartservice.csproj Outdated Show resolved Hide resolved
src/cartservice/otel-agent-config.yaml Outdated Show resolved Hide resolved
kubernetes-manifests/cartservice.yaml Outdated Show resolved Hide resolved
Pin version to 0.28.0 instead of latest in case of future breaking changes.
Remove dev version of image for performance reasons.
Dependencies had been pinned to older versions because of a bug in OpenTelemetry's instrumentation library, which has since been fixed.
Upgraded GRPC version
Remove ß from collector yaml
Copy link

@simonz130 simonz130 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, @arbrown!

Thank you!

kubernetes-manifests/cartservice.yaml Show resolved Hide resolved
@arbrown arbrown merged commit 58fa7c1 into master Jun 15, 2021
@arbrown arbrown deleted the otel-trace branch June 15, 2021 23:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add OpenTelemetry Trace to C# cartservice
4 participants