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(metrics): bump opentelemetry dependenncies #401

Merged
merged 1 commit into from
Feb 18, 2023

Conversation

liufuyang
Copy link
Contributor

Move server.rpc.duration metric to server.rpc.duration_deprecated, as it is now included in the otelgrpc opentelemetry contrib package. A side effect of this is that we loose the status code attr.

Status on this can be tracked here:
open-telemetry/opentelemetry-go-contrib#2840


Basically, do this reverted #382 again as there has been a fix for the memory (infinite loop) issue GoogleCloudPlatform/opentelemetry-operations-go#591

But we keep rpc.server.duration_deprecated metric for now and remove it later. Also allowing a comparison of our own metric implementation vs the opentelemetry-go-contrib implementation

Move server.rpc.duration metric to server.rpc.duration_deprecated,
as it is now included in the otelgrpc opentelemetry contrib
package. A side effect of this is that we loose the status code attr.

Status on this can be tracked here:
open-telemetry/opentelemetry-go-contrib#2840
@ericwenn
Copy link
Member

Why keep the previous metric under new name?

@liufuyang
Copy link
Contributor Author

liufuyang commented Feb 18, 2023

I want to compare whether they have differences as we have some alerts is using it and filtering on grpc status code and so on. If there does have some differences and we have difficulties switching the alerts, we might want to switch back to our internal one for a while, and perhaps able to update the implementation on their side.

@liufuyang liufuyang merged commit 0971d72 into master Feb 18, 2023
@liufuyang liufuyang deleted the update-opentelemetry-operations-go-0.35.2 branch February 18, 2023 09:12
@liufuyang
Copy link
Contributor Author

image
Looks like rpc_grpc_status_code can still be used but rpc_grpc_code will be missing.

@liufuyang
Copy link
Contributor Author

The problem still exists...

image

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