http.server.duration metric includes high cardinality attr: http.client_ip

[vmihailenco]

Meaning that there are as many timeseries as you have visitors. Other http.server.* metrics have the same problem.

Those metrics also include such attrs as http.user_agent, net.sock.peer.addr, net.sock.peer.port, which should also be removed.

[rangzen]

Wrong attributes: https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/instrumentation/net/http/otelhttp/handler.go#L217

[dmathieu]

We should split the httpconv methods to handle traces and metrics differently. Metrics need low cardinality, while the entire point of traces is to have high cardinality.
If we just remove the attributes, that will cause trace issues such as #3743.

[rangzen]

The correct attributes already exist, cf. #3182, and the last comment by @mlaflamm.

Before this commit, attributes were:

attributes := append(labeler.Get(), semconv.HTTPServerMetricAttributesFromHTTPRequest(h.operation, r)...)		

[rangzen]

@MrAlias do you remember why these changes by 7c3b1f6?

[youngbupark]

+1 This results in the high cardinality of request metrics and memory leak in our project. Currently, I am adding the workaround to set req.RemoteAddr to empty before calling otelhttp middleware. But, I hope it will be fixed in the next release.

[rangzen]

Another workaround is to get back to go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.37.0...

[zsgilber]

Hi all, can we get an acknowledgement that this a real bug, and specifically that this current implementation of the metrics does not match the specification, which restricts those high cardinality attributes to just HTTP clients:

[hasheddan]

Ran into this issue, which I imagine most folks are going to hit when upgrading. It is also applicable to otelgrpc (#3071). The cleanest solution I found in the short-term was to utilize views to filter out the high-cardinality attributes. Interestingly the otel-collector folks hit this same issue with its internal metrics. They employed a similar strategy: open-telemetry/opentelemetry-collector#7543

[youngbupark]

Another workaround is to get back to go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.37.0...

It doesn't work for my case. The older one doesn't emit status code I need to use. I am filtering out req.RemoteAddr now.

[davendu]

This is causing issues in our production environment. Our service is not reporting metrics due to the gRPC message too large. I converted the oversized gRPC message into JSON and found otelhttp 0.41.1 is too heavy. The message is then reject by our collector, because it would cause issue for our database.

It seems http.server.request_content_length, http.server.response_content_length and http.server.duration metrics are recording user agent and ip & port info as attributes. It will cause pretty high cardinality, becuase each new attribute requires a whole new bucket, enriching the telemetry message.

Should we consider using a new set of attributes and limit cardinality for metrics?

opentelemetry-go-contrib/instrumentation/net/http/otelhttp/handler.go

Lines 226 to 233 in 65caeb1

	o := metric.WithAttributes(attributes...)
	h.counters[RequestContentLength].Add(ctx, bw.read, o)
	h.counters[ResponseContentLength].Add(ctx, rww.written, o)
	// Use floating point division here for higher precision (instead of Millisecond method).
	elapsedTime := float64(time.Since(requestStartTime)) / float64(time.Millisecond)
	h.valueRecorders[ServerLatency].Record(ctx, elapsedTime, o)

[krak3n]

We've had to implement our own middleware to remove these labels from metrics. There needs to be a clear distinction between attributes that are not safe for metrics but are fine for tracing.

[davendu]

We've had to implement our own middleware to remove these labels from metrics. There needs to be a clear distinction between attributes that are not safe for metrics but are fine for tracing.

dmathieu said in its comment about a feature called "view", being discussed in open-telemetry/opentelemetry-specification#3061, and seems reasonable.

[krak3n]

Thanks @dmathieu

We recently upgraded to the latest metric SDK versions (was a pain 😅) and have started using views. It seems the Go Metric SDK does support a attribute filter function you can apply to a view:

https://pkg.go.dev/go.opentelemetry.io/otel/sdk/metric#Stream
https://pkg.go.dev/go.opentelemetry.io/otel/attribute#Filter

So this should already be possible to achieve and does seem like a nice solution, thanks for the hint 🙌

[jlordiales]

is this still considered an issue with the instrumentation library? Or with the way the semconv package defines client/server attributes?

The spec makes a clear distinction between metric and traces attributes so it feels this instrumentation should as well?

[dprotaso]

I believe this is fixed by #4277 ?

[dmathieu]

Yes.