otelgrpc: Add Filter for stats handler #5196

ymtdzzz · 2024-03-03T07:30:35Z

Description

Add filter for stats handler and move interceptor filter to go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/filters/interceptor considering that it'll be deprecated.

Related Issue

Fixes #4575

Test

Unit tests added.
And I've tested on: https://github.com/grpc/grpc-go/tree/master/examples/route_guide

No filters

Method name filter

	opts = append(opts, grpc.StatsHandler(otelgrpc.NewServerHandler(
		otelgrpc.WithFilter(filters.MethodName("ListFeatures")),
	)))

instrumentation/google.golang.org/grpc/otelgrpc/filters/interceptor/filters.go

CHANGELOG.md

dashpole · 2024-03-04T19:10:31Z

lgtm overall.

codecov · 2024-03-04T19:15:41Z

Codecov Report

Attention: Patch coverage is 88.42105% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 62.7%. Comparing base (d6e2fc4) to head (95bca4e).

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #5196     +/-   ##
=======================================
+ Coverage   62.5%   62.7%   +0.1%     
=======================================
  Files        191     192      +1     
  Lines      11727   11786     +59     
=======================================
+ Hits        7340    7399     +59     
  Misses      4170    4170             
  Partials     217     217

Files	Coverage Δ
...google.golang.org/grpc/otelgrpc/filters/filters.go	`100.0% <100.0%> (+6.0%)`	⬆️
...n/google.golang.org/grpc/otelgrpc/stats_handler.go	`100.0% <100.0%> (ø)`
...entation/google.golang.org/grpc/otelgrpc/config.go	`89.0% <50.0%> (+0.6%)`	⬆️
...g.org/grpc/otelgrpc/filters/interceptor/filters.go	`93.9% <93.9%> (ø)`
...ion/google.golang.org/grpc/otelgrpc/interceptor.go	`89.4% <0.0%> (ø)`

instrumentation/google.golang.org/grpc/otelgrpc/config.go

jmjesperson · 2024-05-03T19:36:25Z

Any update if this will be reviewed in order to be merged?

ymtdzzz · 2024-05-06T01:46:07Z

@dashpole @hanyuancheung
Please let me know if there is any additional work required for this PR to be merged :)

CHANGELOG.md

instrumentation/google.golang.org/grpc/otelgrpc/filters/interceptor/filters.go

instrumentation/google.golang.org/grpc/otelgrpc/config.go

instrumentation/google.golang.org/grpc/otelgrpc/filters/filters.go

CHANGELOG.md

LGTM, but there are some comments that should be addressed.

Co-authored-by: Robert Pająk <pellared@hotmail.com>

pellared · 2024-05-08T12:55:26Z

@dashpole, can you please make another review and possibly merge if you find everything good?

instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go

pgporada · 2024-05-10T19:06:35Z

Hey thanks!

pellared · 2024-05-16T06:49:30Z

@ymtdzzz thank you for your contribution 🥇

* otelgrpc: Add filter for stats handler * Add entries to CHANGELOG * fix CHANGELOG * keep Deprecated comment of `WithInterceptorFilter` * deprecate `InterceptorFilter` type * change the location of `gctx.record` nil check location to reduce heap allocations * add documentation for filters and interceptor packages * fix the location of entries in CHANGELOG * more reasonable doc comment for interceptor package Co-authored-by: Robert Pająk <pellared@hotmail.com> * more reasonable doc comment for interceptor package Co-authored-by: Robert Pająk <pellared@hotmail.com> * Use more appropriate word in the comment of InterceptorFilter Co-authored-by: Robert Pająk <pellared@hotmail.com> * more reasonable doc comment for filters package Co-authored-by: Robert Pająk <pellared@hotmail.com> * fix the comment of `Filter` * Clearly describe the effects of the change * add tests for `stats_handler.go` --------- Co-authored-by: David Ashpole <dashpole@google.com> Co-authored-by: Robert Pająk <pellared@hotmail.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

### Added - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068, #3108) - Add an experimental `OTEL_METRICS_PRODUCERS` environment variable to `go.opentelemetry.io/contrib/autoexport` to be set metrics producers. (#5281) - `prometheus` and `none` are supported values. You can specify multiple producers separated by a comma. - Add `WithFallbackMetricProducer` option that adds a fallback if the `OTEL_METRICS_PRODUCERS` is not set or empty. - The `go.opentelemetry.io/contrib/processors/baggage/baggagetrace` module. This module provides a Baggage Span Processor. (#5404) - Add gRPC trace `Filter` for stats handler to `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (#5196) - Add a repository Code Ownership Policy. (#5555) - The `go.opentelemetry.io/contrib/bridges/otellogrus` module. This module provides an OpenTelemetry logging bridge for `github.com/sirupsen/logrus`. (#5355) - The `WithVersion` option function in `go.opentelemetry.io/contrib/bridges/otelslog`. This option function is used as a replacement of `WithInstrumentationScope` to specify the logged package version. (#5588) - The `WithSchemaURL` option function in `go.opentelemetry.io/contrib/bridges/otelslog`. This option function is used as a replacement of `WithInstrumentationScope` to specify the semantic convention schema URL for the logged records. (#5588) - Add support for Cloud Run jobs in `go.opentelemetry.io/contrib/detectors/gcp`. (#5559) ### Changed - The gRPC trace `Filter` for interceptor is renamed to `InterceptorFilter`. (#5196) - The gRPC trace filter functions `Any`, `All`, `None`, `Not`, `MethodName`, `MethodPrefix`, `FullMethodName`, `ServiceName`, `ServicePrefix` and `HealthCheck` for interceptor are moved to `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/filters/interceptor`. With this change, the filters in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` are now working for stats handler. (#5196) - `NewLogger` now accepts a `name` `string` as the first argument. This parameter is used as a replacement of `WithInstrumentationScope` to specify the name of the logger backing the underlying `Handler`. (#5588) - `NewHandler` now accepts a `name` `string` as the first argument. This parameter is used as a replacement of `WithInstrumentationScope` to specify the name of the logger backing the returned `Handler`. (#5588) - Upgrade all dependencies of `go.opentelemetry.io/otel/semconv/v1.24.0` to `go.opentelemetry.io/otel/semconv/v1.25.0`. (#5605) ### Removed - The `WithInstrumentationScope` option function in `go.opentelemetry.io/contrib/bridges/otelslog` is removed. Use the `name` parameter added to `NewHandler` and `NewLogger` as well as `WithVersion` and `WithSchema` as replacements. (#5588) ### Deprecated - The `InterceptorFilter` type in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` is deprecated. (#5196)

### Added - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (open-telemetry#3068, open-telemetry#3108) - Add an experimental `OTEL_METRICS_PRODUCERS` environment variable to `go.opentelemetry.io/contrib/autoexport` to be set metrics producers. (open-telemetry#5281) - `prometheus` and `none` are supported values. You can specify multiple producers separated by a comma. - Add `WithFallbackMetricProducer` option that adds a fallback if the `OTEL_METRICS_PRODUCERS` is not set or empty. - The `go.opentelemetry.io/contrib/processors/baggage/baggagetrace` module. This module provides a Baggage Span Processor. (open-telemetry#5404) - Add gRPC trace `Filter` for stats handler to `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc`. (open-telemetry#5196) - Add a repository Code Ownership Policy. (open-telemetry#5555) - The `go.opentelemetry.io/contrib/bridges/otellogrus` module. This module provides an OpenTelemetry logging bridge for `github.com/sirupsen/logrus`. (open-telemetry#5355) - The `WithVersion` option function in `go.opentelemetry.io/contrib/bridges/otelslog`. This option function is used as a replacement of `WithInstrumentationScope` to specify the logged package version. (open-telemetry#5588) - The `WithSchemaURL` option function in `go.opentelemetry.io/contrib/bridges/otelslog`. This option function is used as a replacement of `WithInstrumentationScope` to specify the semantic convention schema URL for the logged records. (open-telemetry#5588) - Add support for Cloud Run jobs in `go.opentelemetry.io/contrib/detectors/gcp`. (open-telemetry#5559) ### Changed - The gRPC trace `Filter` for interceptor is renamed to `InterceptorFilter`. (open-telemetry#5196) - The gRPC trace filter functions `Any`, `All`, `None`, `Not`, `MethodName`, `MethodPrefix`, `FullMethodName`, `ServiceName`, `ServicePrefix` and `HealthCheck` for interceptor are moved to `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc/filters/interceptor`. With this change, the filters in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` are now working for stats handler. (open-telemetry#5196) - `NewLogger` now accepts a `name` `string` as the first argument. This parameter is used as a replacement of `WithInstrumentationScope` to specify the name of the logger backing the underlying `Handler`. (open-telemetry#5588) - `NewHandler` now accepts a `name` `string` as the first argument. This parameter is used as a replacement of `WithInstrumentationScope` to specify the name of the logger backing the returned `Handler`. (open-telemetry#5588) - Upgrade all dependencies of `go.opentelemetry.io/otel/semconv/v1.24.0` to `go.opentelemetry.io/otel/semconv/v1.25.0`. (open-telemetry#5605) ### Removed - The `WithInstrumentationScope` option function in `go.opentelemetry.io/contrib/bridges/otelslog` is removed. Use the `name` parameter added to `NewHandler` and `NewLogger` as well as `WithVersion` and `WithSchema` as replacements. (open-telemetry#5588) ### Deprecated - The `InterceptorFilter` type in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` is deprecated. (open-telemetry#5196)

otelgrpc: Add filter for stats handler

3203812

ymtdzzz force-pushed the grpc-stats-filter branch from 4a4eb3e to 3203812 Compare March 3, 2024 08:03

Add entries to CHANGELOG

93304ee

ymtdzzz mentioned this pull request Mar 3, 2024

otelgrpc: stats handler doesn't seem to have an option for filtering #4575

Closed

ymtdzzz commented Mar 3, 2024

View reviewed changes

instrumentation/google.golang.org/grpc/otelgrpc/filters/interceptor/filters.go Show resolved Hide resolved

ymtdzzz changed the title ~~[WIP] otelgrpc: Add Filter for stats handler~~ otelgrpc: Add Filter for stats handler Mar 3, 2024

ymtdzzz marked this pull request as ready for review March 3, 2024 08:13

ymtdzzz requested review from dashpole and hanyuancheung as code owners March 3, 2024 08:13

ymtdzzz requested a review from a team March 3, 2024 08:13

dashpole reviewed Mar 4, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

ymtdzzz added 2 commits March 5, 2024 11:17

fix CHANGELOG

6da878b

Merge branch 'main' into grpc-stats-filter

b2268ee

dashpole reviewed Mar 5, 2024

View reviewed changes

instrumentation/google.golang.org/grpc/otelgrpc/config.go Show resolved Hide resolved

dashpole reviewed Mar 5, 2024

View reviewed changes

instrumentation/google.golang.org/grpc/otelgrpc/config.go Show resolved Hide resolved

ymtdzzz added 2 commits March 6, 2024 18:33

keep Deprecated comment of WithInterceptorFilter

a67ce81

deprecate InterceptorFilter type

bdd913c

pellared requested a review from dashpole March 6, 2024 11:19

dashpole approved these changes Mar 6, 2024

View reviewed changes

pgporada mentioned this pull request Apr 4, 2024

grpc: Replace deprecated otelgrpc interceptors letsencrypt/boulder#7237

Closed

Merge branch 'main' into grpc-stats-filter

f17ccf9

hanyuancheung approved these changes May 6, 2024

View reviewed changes

Merge branch 'main' into grpc-stats-filter

0f3cda4

pellared reviewed May 6, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

pellared mentioned this pull request May 6, 2024

roadmap to make statsHandler stable grpc/grpc-go#4690

Open

pellared previously approved these changes May 6, 2024

View reviewed changes

ymtdzzz and others added 2 commits May 8, 2024 09:34

fix the location of entries in CHANGELOG

733f3be

Merge branch 'open-telemetry:main' into grpc-stats-filter

94aa442

pellared reviewed May 8, 2024

View reviewed changes

ymtdzzz and others added 7 commits May 8, 2024 19:34

more reasonable doc comment for interceptor package

02f36b3

Co-authored-by: Robert Pająk <pellared@hotmail.com>

more reasonable doc comment for interceptor package

e523a2d

Co-authored-by: Robert Pająk <pellared@hotmail.com>

Use more appropriate word in the comment of InterceptorFilter

28f6c1c

Co-authored-by: Robert Pająk <pellared@hotmail.com>

more reasonable doc comment for filters package

a779640

Co-authored-by: Robert Pająk <pellared@hotmail.com>

fix the comment of Filter

ebabbaa

Clearly describe the effects of the change

28a27de

Merge branch 'main' into grpc-stats-filter

79cf21a

pellared approved these changes May 8, 2024

View reviewed changes

pellared requested a review from dashpole May 8, 2024 12:55

dashpole reviewed May 8, 2024

View reviewed changes

instrumentation/google.golang.org/grpc/otelgrpc/stats_handler.go Show resolved Hide resolved

ymtdzzz added 2 commits May 9, 2024 16:41

add tests for stats_handler.go

d22a1c9

Merge branch 'main' into grpc-stats-filter

a7b6221

dashpole approved these changes May 9, 2024

View reviewed changes

Merge branch 'main' into grpc-stats-filter

95bca4e

MrAlias merged commit 0483033 into open-telemetry:main May 9, 2024
23 checks passed

ymtdzzz deleted the grpc-stats-filter branch May 9, 2024 15:12

MrAlias mentioned this pull request May 21, 2024

Release v1.27.0/v0.52.0/v0.21.0/v0.7.0/v0.2.0 #5628

Merged

aarongable mentioned this pull request May 23, 2024

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

Merged

vikmik mentioned this pull request May 23, 2024

Comment TraceLevelInfoFilter as it breaks OTel compatibility gubernator-io/gubernator#21

Merged

renovate bot mentioned this pull request May 24, 2024

Renovate Update Patch & Minor Updates ministryofjustice/opg-go-common#38

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

otelgrpc: Add Filter for stats handler #5196

otelgrpc: Add Filter for stats handler #5196

ymtdzzz commented Mar 3, 2024 •

edited

Loading

dashpole commented Mar 4, 2024

codecov bot commented Mar 4, 2024 •

edited

Loading

jmjesperson commented May 3, 2024

ymtdzzz commented May 6, 2024

pellared commented May 8, 2024

pgporada commented May 10, 2024

pellared commented May 16, 2024

otelgrpc: Add Filter for stats handler #5196

otelgrpc: Add Filter for stats handler #5196

Conversation

ymtdzzz commented Mar 3, 2024 • edited Loading

Description

Related Issue

Test

No filters

Method name filter

dashpole commented Mar 4, 2024

codecov bot commented Mar 4, 2024 • edited Loading

Codecov Report

jmjesperson commented May 3, 2024

ymtdzzz commented May 6, 2024

pellared commented May 8, 2024

pgporada commented May 10, 2024

pellared commented May 16, 2024

ymtdzzz commented Mar 3, 2024 •

edited

Loading

codecov bot commented Mar 4, 2024 •

edited

Loading