Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions examples/features/opentelemetry/client/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ func main() {
// up-to-date list of metrics, see:
// https://grpc.io/docs/guides/opentelemetry-metrics/#instruments
Metrics: opentelemetry.DefaultMetrics().Add(
"grpc.client.attempt.started",
"grpc.client.attempt.duration",
"grpc.xds_client.resource_updates_valid",
Copy link
Member

Choose a reason for hiding this comment

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

Correct me if I am wrong but I dont think we are using xds in this example , so using xds metrics might not make sense. Can you change to use some other metrics? Example : grpc.lb.pick_first. connection_attempts_succeeded

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eshitachandwani , Thanks for the review! I don’t think we need to change this example — we’ve already mentioned that it’s meant to demonstrate experimental metrics. Also, this is aligned with the discussion here: grpc-go#8234 (comment).

Copy link
Member

Choose a reason for hiding this comment

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

But the example in my opinion should be a working example , like if someone is trying to run the example , they should be able to see some metrics. And since the example is not using xds , there will be no metrics for the xds client.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I’ve updated the example to use pick_first with a valid address, and it now emits metrics

"grpc.xds_client.resource_updates_invalid",
),
},
TraceOptions: oteltracing.TraceOptions{TracerProvider: traceProvider, TextMapPropagator: textMapPropagator},
Expand Down
4 changes: 2 additions & 2 deletions examples/features/opentelemetry/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ func main() {
// up-to-date list of metrics, see:
// https://grpc.io/docs/guides/opentelemetry-metrics/#instruments
Metrics: opentelemetry.DefaultMetrics().Add(
"grpc.server.call.started",
"grpc.server.call.duration",
"grpc.xds_client.resource_updates_valid",
"grpc.xds_client.resource_updates_invalid",
),
},
TraceOptions: oteltracing.TraceOptions{TracerProvider: traceProvider, TextMapPropagator: textMapPropagator}})
Expand Down
4 changes: 2 additions & 2 deletions stats/opentelemetry/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ func ExampleOptions_addExperimentalMetrics() {
// up-to-date list of metrics, see:
// https://grpc.io/docs/guides/opentelemetry-metrics/#instruments
Metrics: opentelemetry.DefaultMetrics().Add(
"grpc.client.attempt.started",
"grpc.client.attempt.duration",
"grpc.xds_client.resource_updates_valid",
"grpc.xds_client.resource_updates_invalid",
),
},
}
Expand Down
Loading