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

Replace OpenCensus with OpenTelemetry #2136

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

Conversation

akerouanton
Copy link

@akerouanton akerouanton commented May 6, 2024

This PR is almost ready to review but I'd like to see if/where the CI fails before putting it out of draft mode.


The OpenCensus repo has been archived on July 31, 2023 (see here), and thus will receive no further updates. OTel offers a bridge to help transition however this means every project using hcsshim need to set up that bridge. At this point, it seems better to transition to OTel instead.

This commit fully replaces OpenCensus with OpenTelemetry v1.21.

  • Package internal/oc has been replaced by internal/otelutil.
  • octtrpc has been replaced by otelttrpc.
  • go.opencensus.io/plugin/ocgrpc has been replaced by go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.
  • Whenever possible, span attributes are passed directly when the span is created.

@akerouanton akerouanton force-pushed the replace-opencensus branch from b6f4026 to 78eeeb7 Compare May 6, 2024 20:51
@akerouanton
Copy link
Author

@microsoft-github-policy-service agree

The OpenCensus repo has been archived on July 31, 2023 (see [1]),
and thus will receive no further updates. OTel offers a bridge to
help transition however this means every project using hcsshim
need to set up that bridge. At this point, it seems better to
transition to OTel instead.

This commit fully replaces OpenCensus with OpenTelemetry v1.21.

- Package `internal/oc` has been replaced by `internal/otelutil`.
- `octtrpc` has been replaced by `otelttrpc`.
- `go.opencensus.io/plugin/ocgrpc` has been replaced by `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`.
- Whenever possible, span attributes are passed directly when the
span is created.

[1]: https://opentelemetry.io/blog/2023/sunsetting-opencensus/

Signed-off-by: Albin Kerouanton <[email protected]>
Previous commit made clear that `(*externalProcess).Wait()` was
missing a call to `span.End()`.

Signed-off-by: Albin Kerouanton <[email protected]>
@akerouanton akerouanton force-pushed the replace-opencensus branch from 78eeeb7 to 136345a Compare May 7, 2024 06:21
@akerouanton akerouanton marked this pull request as ready for review May 21, 2024 13:53
@akerouanton akerouanton requested a review from a team as a code owner May 21, 2024 13:53
@kevpar
Copy link
Member

kevpar commented Jul 5, 2024

Hi @akerouanton, thanks for sending this PR!

Currently we have some internal usage of the OC spans, so I'd like to make sure we have a smooth transition from OC to OTel to ensure nothing breaks. So, my current thinking is we should do something like the following:

  • Abstract all current OC span creation (maybe internal/span) behind an internal package, such that no OC specific packages need to be imported in most of the code.
  • In internal/span, add support to export to OTel. Have a package config setting that controls whether spans are exported to OC, OTel, or (potentially) both.

With this in place, we'd be able to get OTel support merged, but keep internal usage going to OC until we can validate that the OTel work won't break anything.

As a plus, this approach also gives us a way to potentially inject other span-wide instrumentation, such as using pprof.WithLabels.

Let me know if you have thoughts on this approach.

@i02sopop
Copy link

Hi,

The OpenCensus certificate has expired, so the dependency can't get downloaded anymore with a regular go mod tidy, and impacts on other repositories. Any plans to merge this PR?

go mod tidy                                                                                                                              [18:23:23]
go: downloading go.opencensus.io v0.24.0
go: XXX/internal/client/helm imports
        helm.sh/helm/v3/pkg/registry imports
        oras.land/oras-go/pkg/content imports
        github.com/containerd/containerd/content/local tested by
        github.com/containerd/containerd/content/local.test imports
        github.com/containerd/containerd/pkg/testutil imports
        github.com/containerd/containerd/mount imports
        github.com/Microsoft/hcsshim imports
        github.com/Microsoft/hcsshim/internal/hcs imports
        go.opencensus.io/trace: unrecognized import path "go.opencensus.io": https fetch: Get "https://go.opencensus.io/?go-get=1": tls: failed to verify certificate: x509: certificate has expired or is not yet valid: current time 2025-01-22T18:23:26+01:00 is after 2025-01-21T03:43:04Z```

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.

3 participants