From d52891821bd4dfc727f959726443f8a5cab4bdfb Mon Sep 17 00:00:00 2001 From: Ziqi Zhao Date: Fri, 7 Oct 2022 14:27:22 +0800 Subject: [PATCH] fix some review comments Signed-off-by: Ziqi Zhao --- CHANGELOG.md | 10 ++- .../google.golang.org/grpc/otelgrpc/config.go | 64 +-------------- .../grpc/otelgrpc/metadata_supplier.go | 81 +++++++++++++++++++ 3 files changed, 91 insertions(+), 64 deletions(-) create mode 100644 instrumentation/google.golang.org/grpc/otelgrpc/metadata_supplier.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 933ad3a848f..b5dcfe4fb4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,16 +8,19 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] - ### Changed - google.golang.org/grpc/otelgrpc: Avoid getting a new Tracer for every RPC. +### Added + +- [otelgrpc] Add `WithMeterProvider` function to enable metric and add metric `rpc.server.duration` to otelgrpc instrumentation library. (#2700) + ## [0.36.1] ### Changed -- Upgrade dependencies of the OpenTelemetry Go Metric SDK to use the new [`v0.32.1` release](https://github.com/open-telemetry/opentelemetry-go/releases +- Upgrade dependencies of the OpenTelemetry Go Metric SDK to use the new [`v0.32.1` release](https://github.com/open-telemetry/opentelemetry-go/releases) ### Removed @@ -37,6 +40,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Rename the `Typ` field of `"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc".InterceptorInfo` to `Type`. (#2688) - Use Go 1.19 as the default version for CI testing/linting. (#2675) +<<<<<<< HEAD ### Added - [otelgrpc] add metric `rpc.server.duration` to otelgrpc instrumentation library. (#2700) @@ -47,6 +51,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm Also fix the propagator rejecting span IDs shorter than 64 bits. This fixes compatibility with Jaeger clients encoding trace and span IDs as variable-length hex strings, [as required by the Jaeger propagation format](https://www.jaegertracing.io/docs/1.37/client-libraries/#value). (#2731) +======= +>>>>>>> 14e51e40 (fix some review comments) ## [1.9.0/0.34.0/0.4.0] - 2022-08-02 ### Added diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/config.go b/instrumentation/google.golang.org/grpc/otelgrpc/config.go index 677db48e98c..5070b8ac6cb 100644 --- a/instrumentation/google.golang.org/grpc/otelgrpc/config.go +++ b/instrumentation/google.golang.org/grpc/otelgrpc/config.go @@ -15,17 +15,13 @@ package otelgrpc // import "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" import ( - "context" - - "google.golang.org/grpc/metadata" - "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/baggage" "go.opentelemetry.io/otel/metric" "go.opentelemetry.io/otel/metric/global" "go.opentelemetry.io/otel/metric/instrument/syncfloat64" "go.opentelemetry.io/otel/propagation" + semconv "go.opentelemetry.io/otel/semconv/v1.12.0" "go.opentelemetry.io/otel/trace" ) @@ -71,6 +67,7 @@ func newConfig(opts []Option) *config { c.meter = c.MeterProvider.Meter( instrumentationName, metric.WithInstrumentationVersion(SemVersion()), + metric.WithSchemaURL(semconv.SchemaURL), ) var err error if c.rpcServerDuration, err = c.meter.SyncFloat64().Histogram("rpc.server.duration"); err != nil { @@ -136,60 +133,3 @@ func (o meterProviderOption) apply(c *config) { func WithMeterProvider(mp metric.MeterProvider) Option { return meterProviderOption{mp: mp} } - -type metadataSupplier struct { - metadata *metadata.MD -} - -// assert that metadataSupplier implements the TextMapCarrier interface. -var _ propagation.TextMapCarrier = &metadataSupplier{} - -func (s *metadataSupplier) Get(key string) string { - values := s.metadata.Get(key) - if len(values) == 0 { - return "" - } - return values[0] -} - -func (s *metadataSupplier) Set(key string, value string) { - s.metadata.Set(key, value) -} - -func (s *metadataSupplier) Keys() []string { - out := make([]string, 0, len(*s.metadata)) - for key := range *s.metadata { - out = append(out, key) - } - return out -} - -// Inject injects correlation context and span context into the gRPC -// metadata object. This function is meant to be used on outgoing -// requests. -func Inject(ctx context.Context, md *metadata.MD, opts ...Option) { - c := newConfig(opts) - inject(ctx, md, c.Propagators) -} - -func inject(ctx context.Context, md *metadata.MD, propagators propagation.TextMapPropagator) { - propagators.Inject(ctx, &metadataSupplier{ - metadata: md, - }) -} - -// Extract returns the correlation context and span context that -// another service encoded in the gRPC metadata object with Inject. -// This function is meant to be used on incoming requests. -func Extract(ctx context.Context, md *metadata.MD, opts ...Option) (baggage.Baggage, trace.SpanContext) { - c := newConfig(opts) - return extract(ctx, md, c.Propagators) -} - -func extract(ctx context.Context, md *metadata.MD, propagators propagation.TextMapPropagator) (baggage.Baggage, trace.SpanContext) { - ctx = propagators.Extract(ctx, &metadataSupplier{ - metadata: md, - }) - - return baggage.FromContext(ctx), trace.SpanContextFromContext(ctx) -} diff --git a/instrumentation/google.golang.org/grpc/otelgrpc/metadata_supplier.go b/instrumentation/google.golang.org/grpc/otelgrpc/metadata_supplier.go new file mode 100644 index 00000000000..fcd4f53665a --- /dev/null +++ b/instrumentation/google.golang.org/grpc/otelgrpc/metadata_supplier.go @@ -0,0 +1,81 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package otelgrpc // import "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" + +import ( + "context" + + "go.opentelemetry.io/otel/baggage" + "go.opentelemetry.io/otel/propagation" + "go.opentelemetry.io/otel/trace" + "google.golang.org/grpc/metadata" +) + +type metadataSupplier struct { + metadata *metadata.MD +} + +// assert that metadataSupplier implements the TextMapCarrier interface. +var _ propagation.TextMapCarrier = &metadataSupplier{} + +func (s *metadataSupplier) Get(key string) string { + values := s.metadata.Get(key) + if len(values) == 0 { + return "" + } + return values[0] +} + +func (s *metadataSupplier) Set(key string, value string) { + s.metadata.Set(key, value) +} + +func (s *metadataSupplier) Keys() []string { + out := make([]string, 0, len(*s.metadata)) + for key := range *s.metadata { + out = append(out, key) + } + return out +} + +// Inject injects correlation context and span context into the gRPC +// metadata object. This function is meant to be used on outgoing +// requests. +func Inject(ctx context.Context, md *metadata.MD, opts ...Option) { + c := newConfig(opts) + inject(ctx, md, c.Propagators) +} + +func inject(ctx context.Context, md *metadata.MD, propagators propagation.TextMapPropagator) { + propagators.Inject(ctx, &metadataSupplier{ + metadata: md, + }) +} + +// Extract returns the correlation context and span context that +// another service encoded in the gRPC metadata object with Inject. +// This function is meant to be used on incoming requests. +func Extract(ctx context.Context, md *metadata.MD, opts ...Option) (baggage.Baggage, trace.SpanContext) { + c := newConfig(opts) + return extract(ctx, md, c.Propagators) +} + +func extract(ctx context.Context, md *metadata.MD, propagators propagation.TextMapPropagator) (baggage.Baggage, trace.SpanContext) { + ctx = propagators.Extract(ctx, &metadataSupplier{ + metadata: md, + }) + + return baggage.FromContext(ctx), trace.SpanContextFromContext(ctx) +}