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

fix: downgrade opentelemetry-operations-go to 0.34.1 #395

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

liufuyang
Copy link
Contributor

@liufuyang liufuyang commented Feb 14, 2023

as we see memory leak type of issue and a constant increase of bytes to google with updated version of opentelemetry-operations-go.

This reverts some of the changes made here #382

The issue seen from a backend service could be like:

vehicle-service
image

own-service
image

vehicle-service CPU profile
image

https://console.cloud.google.com/profiler;timespan=1h;end=2023-02-14T11:30:00.000Z/vehicle-eu/cpu?project=e-vehicle-service-prod&pli=1

Also could see the backend service's metric will break when updated...
image

as we see memory leak type of issue and a constant
increase of bytes to google with updated version of
opentelemetry-operations-go
@@ -40,8 +40,8 @@ require (
cloud.google.com/go/compute v1.18.0 // indirect
cloud.google.com/go/monitoring v1.12.0 // indirect
cloud.google.com/go/trace v1.8.0 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.11.1 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.35.1 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.11.0 // indirect
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried to put this to 1.10.1 but go mod tidy updates it.

@liufuyang
Copy link
Contributor Author

Some other packages needed to downgrade as well to fix build error:

Error: ../../../go/pkg/mod/github.com/!google!cloud!platform/opentelemetry-operations-go/exporter/metric@v0.34.1/cloudmonitoring.go:48:9: cannot use newMetricExporter(&o) (value of type *metricExporter) as type "go.opentelemetry.io/otel/sdk/metric".Exporter in return statement:
*metricExporter does not implement "go.opentelemetry.io/otel/sdk/metric".Exporter (missing Aggregation method)

@liufuyang liufuyang force-pushed the downgrade-opentelemetry-operations-go branch from 8db1446 to afd72a6 Compare February 14, 2023 16:23
@thall
Copy link
Member

thall commented Feb 14, 2023

Cant we just revert #382 ?

@liufuyang
Copy link
Contributor Author

@thall This is more or less a manual revert of #382 but only touch on the places that need to be reverted.

We can perhaps revert #382 as well but I guess it also needs to revert #394 at the same time?

@thall
Copy link
Member

thall commented Feb 14, 2023

I think its easier just revert the whole PR rather than trying to cherry-pick stuff from it.
Oh you have already merged one PR, either you revert that and then the original, or you continue the path you are on.

@liufuyang
Copy link
Contributor Author

Tried to revert one by one from the end but got some conflicts... Will not touch it for now, and feel free to make another PR on your side to fix it if you find a good/easy way to revert those commits back :)

@liufuyang liufuyang merged commit 89c052d into master Feb 15, 2023
@liufuyang liufuyang deleted the downgrade-opentelemetry-operations-go branch February 15, 2023 14:00
@liufuyang
Copy link
Contributor Author

We can try update again when this GoogleCloudPlatform/opentelemetry-operations-go#591 is merged

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.

None yet

3 participants