go-cloud
go-cloud copied to clipboard
server/*: migrate server tracing to OpenTelemetry
This PR is a step toward migrating Tracing and Metrics from OpenCensus to OpenTelemetry. discussed at #2877
Packages changed are: server server/xrayserver server/sdserver internal/trace
Please rescan for CLA to take effect
Sorry, I'm not super familiar with OpenCensus or OpenTelemetry.
Is this a backwards-compatible change?
@saeidakbari can you rebase this and sign the CLA?
Sorry for the very late check on this PR. These changes are not backward compatiable. Is it OK to proceed?
I don't see how it's possible to make this change if it's not backwards compatible. Wouldn't it potentially break all existing users?
(Doesn't OpenTelemetry have a compatibility story with OpenCensus?)
Practically yes, OpenTelemetry is a successor of OpenCensus but the minimum Go version required for it is 1.16.
The minimum Go version is not a problem.
I think https://opentelemetry.io/docs/migration/opentracing/ describes how to make it backward compatible, by having the OpenTelemetry code also emit OpenCensus events.
OK, then I think there isn't any issue to continue this PR. I will make some changes in the next few days.
@vangent Conflict resolved, but this PR only change the server package to OpenTelemetry. I did it with the purpose of not making a big PR with lots of code changes.
Related question: why are you motivated to make this change? Can't you use the bridge to get OpenTelemetry events from this library?
@vangent Accessing OpenCensus data is possible through migrating to OpenTelemetry and implementing the bridge but not vice versa. Basically, OpenTelemetry is the future of tracing, metrics and monitoring and migrating to it can bring new functionalities in different aspects.
Yes, I think that agrees with what I said. What functionality do you expect to get by doing the migration in this library that you wouldn't get by using the bridge?
My understanding of the compatibility requirements as stated above imply that shared libraries like this one should continue emitting OpenCensus events, supporting both their OpenCensus and OpenTelemetry users, until such a point where there are basically no more OpenCensus users.
I'm not sure how to determine when that point is unfortunately, any ideas? Maybe I can make an announcement that we're planning on making the non-backward-compatible change as part of the next release announcement and see if anyone objects.
Mostly third-party libraries are more available and maintained for OpenTelemetry rather than deprecated OpenCensus. Also, this migration is inevitable as the process of migration has to happen sometime.
As this project appears to be used in many different places and has a good reputation I think an announcement before the non-backwards change would be good to see if we have any complaints.
Related question: why are you motivated to make this change? Can't you use the bridge to get OpenTelemetry events from this library?
We depend on go-cloud,and this is causing the fuzz testing to fail in oss-fuzz
https://github.com/ossf/scorecard-webapp/issues/220
I did a release today, and included an announcement that we'll be moving to OpenTelemetry in the next release. Unless there are objections over the next couple of weeks, I'll re-review this and plan to get it merged in after that. Thanks for your patience!
Note that I believe you can use the links in comments above to get OpenCensus libraries to emit OpenTelemetry events.
Let's go ahead with this. Can you sync to HEAD?
A friendly ping.
Cloned to #3189 , but tests don't pass.