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

Remove code for Jaeger integration #947

Closed
ahrarmonsur opened this issue Aug 9, 2022 · 2 comments
Closed

Remove code for Jaeger integration #947

ahrarmonsur opened this issue Aug 9, 2022 · 2 comments
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.

Comments

@ahrarmonsur
Copy link
Contributor

Describe request or inquiry

The repo includes unused code and YAML configs related to Jaeger tracing, which has been replaced by showcasing Google Cloud Operations. As such, Jaeger-related code and documentation should be deprecated and removed.

What purpose/environment will this feature serve?

Since we now showcase Google CloudOps has our tracing agent, and we don't maintain a Jaeger variant of OnlineBoutique, it would make sense to remove the related code from the main stream.

From @askmeegs:

The jaeger support in Online Boutique predates my time at Google, but my understanding is that it was designed as an OSS integration for tracing if the user didn't want to use Cloud Tracing (formerly Stackdriver Tracing).
But right now the toggles for tracing enable/disable both Jaeger and Cloud Tracing at the same time. 🤷🏼 https://github.com/GoogleCloudPlatform/microservices-demo/blob/main/src/shippingservice/main.go#L206
I think given that Online Boutique is a first party GCP demo designed to showcase GCP, it's okay to remove Jaeger from the upstream.

@minherz minherz added type: cleanup An internal cleanup or hygiene concern. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Aug 9, 2022
@mathieu-benoit
Copy link
Contributor

Any advice from @minherz or @arbrown?

Also, just making sure we have visibility of this, we have 2 issues open regarding Jaeger:

@mathieu-benoit
Copy link
Contributor

mathieu-benoit commented Oct 3, 2022

Closing this one as Jaeger has been removed from the project: #1088, coming in 0.3.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: cleanup An internal cleanup or hygiene concern.
Projects
None yet
Development

No branches or pull requests

3 participants