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.

Sign up for GitHub

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

Jump to bottom

Add metrics instrumentation for google.golang.org/grpc #2840

Open
1 of 20 tasks
fatsheep9146 opened this issue Oct 7, 2022 · 5 comments
Open
1 of 20 tasks

Add metrics instrumentation for google.golang.org/grpc #2840

fatsheep9146 opened this issue Oct 7, 2022 · 5 comments
Labels
area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelgrpc

Comments

@fatsheep9146
Copy link
Contributor

fatsheep9146 commented Oct 7, 2022 •
edited

Background

google.golang.org/grpc is already instrumented with library
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc, but this library only covers trace.

So this issue will try to complete all the metrics for google.golang.org/grpc which are already defined in https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/metrics/semantic_conventions/rpc-metrics.md

Metrics

Instruments

Server

Client

  • rpc.client.duration
  • rpc.client.request.size
  • rpc.client.response.size
  • rpc.client.requests_per_rpc
  • rpc.client.responses_per_rpc

Tasks

  • Code complete:
    • Comprehensive unit tests.
    • End-to-end integration tests.
    • Tests all passing.
    • Instrumentation functionality verified.
  • Documented
  • Examples
    • Dockerfile file to build example application.
    • docker-compose.yml to run example in a docker environment to demonstrate instrumentation.
@fatsheep9146 fatsheep9146 added area: instrumentation Related to an instrumentation package enhancement New feature or request labels Oct 7, 2022
@fatsheep9146 fatsheep9146 mentioned this issue Oct 7, 2022
Merged
@dmathieu
Copy link
Member

dmathieu commented Oct 7, 2022

Related: #2782.

@MrAlias MrAlias mentioned this issue Nov 3, 2022
Open
6 tasks
radhus added a commit to einride/cloudrunner-go that referenced this issue Nov 4, 2022
@radhus
fix(cloudmonitoring): disable latency metrics
fb89472 
The exported histograms do not make sense. It seems like they are of
CUMULATIVE type instead of DELTA. This has to be fixed within the
opentelemetry-operations-go library.

As there might be gRPC metrics introduced in the
opentelemetry-go-contrib project (see [1]), let's not try to fix them
here right now but instead remove it and use future ones in that
library.

[1]: open-telemetry/opentelemetry-go-contrib#2840
radhus added a commit to einride/cloudrunner-go that referenced this issue Nov 4, 2022
@radhus
fix(cloudmonitoring): remove latency metrics
3ed6d15 
The exported histograms do not make sense. It seems like they are of
CUMULATIVE type instead of DELTA. This has to be fixed within the
opentelemetry-operations-go library.

As there might be gRPC metrics introduced in the
opentelemetry-go-contrib project (see [1]), let's not try to fix them
here right now but instead remove it and use future ones in that
library.

[1]: open-telemetry/opentelemetry-go-contrib#2840
@radhus radhus mentioned this issue Nov 4, 2022
Closed
@fatsheep9146
Copy link
Contributor Author

I think this issue is now blocked by #197

@fatsheep9146 fatsheep9146 mentioned this issue Nov 9, 2022
Closed
@fatsheep9146 fatsheep9146 mentioned this issue Dec 8, 2022
Merged
Edholm added a commit to einride/cloudrunner-go that referenced this issue Feb 7, 2023
@Edholm
fix(cloudmonitoring): remove server.rpc.duration metric
ed08d01 
This metric 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
@Edholm Edholm mentioned this issue Feb 7, 2023
Merged
liufuyang added a commit to einride/cloudrunner-go that referenced this issue Feb 18, 2023
@liufuyang
fix(metrics): bump opentelemetry dependenncies
d9bd400 
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
@liufuyang liufuyang mentioned this issue Feb 18, 2023
Merged
liufuyang added a commit to einride/cloudrunner-go that referenced this issue Feb 18, 2023
@liufuyang
fix(metrics): bump opentelemetry dependenncies
0971d72 
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
@pellared pellared changed the title Request to Add Instrumentation for google.golang.org/grpc Add metrics instrumentation for google.golang.org/grpc Aug 10, 2023
@Sovietaced
Copy link
Contributor

Anyone mind if I add a metrics implementation to the client interceptors?

@ash2k
Copy link
Contributor

ash2k commented Sep 26, 2023

I think interceptors are going to be deprecated soon and then removed. #4318, #4319. Stats handlers are better - it's hard to do metrics with streaming RPCs as that would require wrapping the stream. It's very clean with stats handlers as gRPC just calls its methods.

@Sovietaced
Copy link
Contributor

I think interceptors are going to be deprecated soon and then removed. #4318, #4319. Stats handlers are better - it's hard to do metrics with streaming RPCs as that would require wrapping the stream. It's very clean with stats handlers as gRPC just calls its methods.

Makes sense, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelgrpc
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@dmathieu @ash2k @Sovietaced @MrAlias @fatsheep9146