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.

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

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

Open
pembletonj2-sonrai opened this issue Aug 20, 2024 · 3 comments
Labels
area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelgrpc

Comments

@pembletonj2-sonrai
Copy link

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.

@pembletonj2-sonrai pembletonj2-sonrai added area: instrumentation Related to an instrumentation package enhancement New feature or request instrumentation: otelgrpc labels Aug 20, 2024
@dmathieu
Copy link
Member

Similar solution for the otelhttp instrumentation: #5876

@dhowden
Copy link

dhowden commented Sep 5, 2024

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): #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
Copy link
Member

scorpionknifes commented Sep 26, 2024

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.

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

No branches or pull requests

4 participants