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

all : Opentelemetry migration away from opencensus #3539

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

pitabwire
Copy link

Pr is an effort to resolve issue #2877 all use of opencensus has been replaced with opentelemetry.

@vangent
Copy link
Contributor

vangent commented Apr 5, 2025

Thanks for taking this on! This PR is too big for me to be able to usefully review it. Is there any reason we can't do this migration one package at a time? Maybe start with an easy one like secrets.

@pitabwire
Copy link
Author

@vangent other than time constraints, getting the tests to work was fairly non trivial when you maintain a compatibility shim. The same application needs to be monitored by two different services together hence the decision to just make everything switched at once.

@vangent
Copy link
Contributor

vangent commented Apr 5, 2025

Sorry, I'm not sure I follow your comment.

  • What are the implications for this change on Go CDK users? Will they have to change their monitoring setup, or is the change backwards compatible?
  • Why can't we make the change a package at a time? I can hold off on doing a release until they are all done.

@pitabwire
Copy link
Author

Sorry, I'm not sure I follow your comment.

@vangent that is fine

  • What are the implications for this change on GoCDK users? Will they have to change their monitoring setup, or is the change backwards compatible?
  • This change is not backwards compatible for GoCDK users as they are different projects.
  • The monitoring solutions are not changed however how data is exported generally will require some change
  • Opencensus is being sunset see here the go version of OC repository is also already archived.
  • Opencensus has alot of direct integrations to APM providers from within the code because of fragmented monitoring service APIs hence it was important to maintain alot of different custom sdks to support the different providers.
  • Opentelemetry on the other hand is more standardized around OTLP only requiring minimal changes to the monitoring backend endpoint the trace and metrics exporter points to.
  • AWS users may have to do some work as xray doesn't have a compliant OTLP endpoint like other providers. They recommend using the ADOT collector](https://aws-otel.github.io/docs/introduction) alongside the app which is not very intuitive like say Google cloud trace which has not much friction.
  • Back to GoCDK, it is a well written project with tonnes of tests to verify functionality including those of the tracing functionality, these were the guard rails and had to pass before I even considered openning this PR.
  • Why can't we make the change a package at a time? I can hold off on doing a release until they are all done.
  • This is just a matter of lack of enough resources and time to continue working on this feature. As it is I have spent too much time on it already since last year. Starting all over with a compatibility shim and ensuring the tests are ok will require more effort in my opinion than spending more time reviewing. Some help from the community to get it over the hump is really welcome.

@vangent
Copy link
Contributor

vangent commented Apr 6, 2025

We can do this a package at a time. You say that will require starting all over with a compatibility shim -- I'm not suggesting that., just splitting this PR into smaller ones. I am not worried about the intermediate state where some packages are using OpenCensus and others are using OpenTelemetry -- I can hold off on making a release until the transition is complete.

Let's start with the internal supporting files -- please just send one PR that adds otel/* and oteltest/* and I can review that. Once that PR is done, we can do the remaining packages (blob, secrets, server, etc.) each separately, in parallel if you want.

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

Successfully merging this pull request may close these issues.

2 participants