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

update otel dependencies to v1.27.0 and v0.52.0 #7496

Merged

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github May 22, 2024

Directly update:

  • go.opentelemetry.io/otel/* from v1.26.0 to v1.27.0
  • go.opentelemetry.io/contrib/* from v0.51.0 to v0.52.0

Indirectly update:

  • google.golang.org/protobuf from v1.33.0 to v1.34.0

This update breaks some of our existing otel grpc interceptors, but in return allows us to use the newer grpc StatsHandler mechanism, while still filtering out health-check requests.

Fixes #7235

@dependabot dependabot bot requested a review from a team as a code owner May 22, 2024 01:52
@dependabot dependabot bot added dependencies Pull requests that update a dependency file go Pull requests that update Go code labels May 22, 2024
@dependabot dependabot bot requested a review from beautifulentropy May 22, 2024 01:52
@beautifulentropy beautifulentropy requested review from pgporada, aarongable and a team May 22, 2024 19:51
Copy link
Contributor

@aarongable aarongable left a comment

Choose a reason for hiding this comment

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

Let's have this PR update go.opentelemetry.io/contrib to v0.52.0 at the same time. That's part of the same bundled release, and contains the change we need to finish #7235

@beautifulentropy beautifulentropy force-pushed the dependabot/go_modules/go.opentelemetry.io/otel/trace-1.27.0 branch from 759f970 to 8cd5eea Compare May 28, 2024 14:03
@beautifulentropy
Copy link
Member

@dependabot rebase

updated-dependencies:
- dependency-name: go.opentelemetry.io/otel/trace
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot dependabot bot force-pushed the dependabot/go_modules/go.opentelemetry.io/otel/trace-1.27.0 branch from 8cd5eea to 4d4ee28 Compare May 28, 2024 14:05
@beautifulentropy beautifulentropy force-pushed the dependabot/go_modules/go.opentelemetry.io/otel/trace-1.27.0 branch from d741b5f to 4d4ee28 Compare May 28, 2024 15:50
@beautifulentropy
Copy link
Member

beautifulentropy commented May 28, 2024

Let's have this PR update go.opentelemetry.io/contrib to v0.52.0 at the same time. That's part of the same bundled release, and contains the change we need to finish #7235

After spending some time on this it appears that an update to v0.52.0 is going to be a heavy lift that should happen in its own PR. Upgrading this dependency necessitates an update to several others and ultimately breaks the build due to an otel schema change:

Dependencies:

	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.52.0
	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.52.0
	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.27.0
	go.opentelemetry.io/otel/sdk v1.27.0
        google.golang.org/protobuf v1.34.1

Build issue post go mod tidy && go mod vendor:

# github.com/letsencrypt/boulder/grpc
grpc/server.go:157:66: cannot use filters.Not(filters.HealthCheck()) (value of type otelgrpc.Filter) as otelgrpc.InterceptorFilter value in argument to otelgrpc.WithInterceptorFilter

Fix for build issue:

	options := []grpc.ServerOption{
		grpc.Creds(creds),
		grpc.ChainUnaryInterceptor(unaryInterceptors...),
		grpc.ChainStreamInterceptor(streamInterceptors...),
		grpc.StatsHandler(otelgrpc.NewServerHandler(
			otelgrpc.WithFilter(
				filters.Not(filters.HealthCheck()))),
		),
	}
Starting service remoteva-a
16:00:05.054880 3 remoteva 6MuMzQY [AUDIT] Could not create OpenTelemetry resource: conflicting Schema URL: https://opentelemetry.io/schemas/1.25.0 and https://opentelemetry.io/schemas/1.24.0
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1000 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1001 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1001 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1001 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1000 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1001 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1000 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1001 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 1000 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 885 ms
health checking rva.boulder (localhost:9897)
health checking rva.boulder (localhost:9897): grpc.health.v1.Health.Check timed out after 0 ms
16:00:15.045724 3 health-checker 7d6YxQY [AUDIT] timed out waiting for localhost:9897 health check

If you find a way to do this without the heavier lift, I'm all for it, but I'm not seeing an easy win here.

@beautifulentropy beautifulentropy self-requested a review May 28, 2024 16:02
@aarongable
Copy link
Contributor

I'll do it later today. I don't see any point to upgrade to 1.27 here without getting the accompanying 0.52 upgrade that we actually want, and the schema URL is just the same thing as I fixed in the previous PR here.

It sucks that these packages are structured such that dependabot can't simply update them correctly.

@aarongable aarongable changed the title build(deps): bump go.opentelemetry.io/otel/trace from 1.26.0 to 1.27.0 update otel dependencies to v1.27.0 and v0.52.0 May 28, 2024
@aarongable
Copy link
Contributor

Okay, updated the PR to also update the other dependencies, added the fixes for the API breaking changes, and updated the commit description. PTAnotherL!

@beautifulentropy
Copy link
Member

I'll do it later today. I don't see any point to upgrade to 1.27 here without getting the accompanying 0.52 upgrade that we actually want, and the schema URL is just the same thing as I fixed in the previous PR here.

It sucks that these packages are structured such that dependabot can't simply update them correctly.

Neat, I'm happy you were able to figure out the schema URL issue.

@beautifulentropy beautifulentropy requested a review from jsha May 28, 2024 20:06
@aarongable aarongable merged commit 6b4577e into main May 29, 2024
12 checks passed
@aarongable aarongable deleted the dependabot/go_modules/go.opentelemetry.io/otel/trace-1.27.0 branch May 29, 2024 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace deprecated otelgrpc interceptors with stats handler
3 participants