open-telemetry / opentelemetry-go-contrib

Collection of extensions for OpenTelemetry-Go.
https://opentelemetry.io/
Apache License 2.0
1.21k stars 564 forks source link

otelgrpc: Allow adding custom attributes to a request's metrics based on its payload #6026

Open pembletonj2-sonrai opened 2 months ago

pembletonj2-sonrai commented 2 months ago

Problem Statement

It's not possible to add custom attributes to the metrics generated by otelgrpc based on the data in the request payloads.

Proposed Solution

A new option can be added to pass a function which will be run on each payload received to generate metric attributes for that request. Example:

otelgrpc.NewServerHandler(otelgrpc.WithMetricAttributesFromPayload(func(payload any) []attribute.KeyValue {
    data1 := extractSomeDataFromPayload(payload)
    data2 := extractOtherDataFromPayload(payload)
    return []attribute.KeyValue{
        attribute.String("key1", data1),
        attribute.String("key2", data2),
    }
}))

key1 and key2 will then be included in the metrics' attributes for this request.

Alternatives

We found an alternative way to add the needed attributes to spans. The span's attributes can be modified from an interceptor:

span := trace.SpanFromContext(ctx)
span.SetAttributes(attribute.String("key", value))

The payload can also be accessed from an interceptor, so its data can be used to set a span's attribute. However, it's not possible to do this with metrics.

Prior Art

3894 discusses setting constant values for metric/span attributes and a PR has been made for it: #5133. This issue builds on this by allowing custom attributes to be computed on a per-request basis rather than being set to a constant value.

dmathieu commented 2 months ago

Similar solution for the otelhttp instrumentation: https://github.com/open-telemetry/opentelemetry-go-contrib/pull/5876

dhowden commented 2 months ago

Looking at the HTTP implementation (#5876), feels like we can make a very similar change, but also passing the incoming ctx too (so that callers can extract metadata etc).

The associated option then might look like:

func WithMetricAttributesFn(metricAttributesFn func(ctx context.Context,  payload any) []attribute.KeyValue) Option {
// ...
}

I've made a sketch implementation here (in draft): https://github.com/open-telemetry/opentelemetry-go-contrib/pull/6092, mostly because the grpc.StatsHandler pattern makes it a bit messier and there might be strong opinions about how this is wired in. Happy to carry this through if there is consensus on this approach.

scorpionknifes commented 1 month ago

Took a quick look, just wondering, why not use stats.RPCStats or *stats.InPayload instead of just passing payload any?

MetricAttributesFn func(ctx context.Context, rpcStats stats.RPCStats) []attribute.KeyValue
// or just *stats.InPayload if we scoping the feature to only request
MetricAttributesInPayloadFn func(ctx context.Context, inPayload *stats.InPayload) []attribute.KeyValue

However I see the issue of with developer experience for stats.RPCStats which requires the user to type casting. We might want something more complex (some of these might be out of scope).

MetricAttributesInPayloadFn func(ctx context.Context, inPayload *stats.InPayload) []attribute.KeyValue
MetricAttributesOutPayloadFn func(ctx context.Context, outPayload *stats.OutPayload) []attribute.KeyValue
MetricAttributesOutHeaderFn func(ctx context.Context, inPayload *stats.OutHeader) []attribute.KeyValue
MetricAttributesEndFn func(ctx context.Context, inPayload *stats.End) []attribute.KeyValue

Also what's the purpose of passing the ctx in? I feel like we can achieve the ctx metadata -> attributes with something else

Just dropping some initial thoughts for discussion.