[Feature]: Don´t hardcode Bucket Boundaries

[ngehrsitz]

Component

Instrumentation: otelhttp

Problem Statement

At the moment bucket boundaries are hardcoded with a maximum value of 10s:

opentelemetry-go-contrib/instrumentation/net/http/otelhttp/internal/semconv/client.go

Lines 38 to 41 in c4223d5

	client.requestDuration, err = httpconv.NewClientRequestDuration(
	meter,
	metric.WithExplicitBucketBoundaries(0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10),
	)

opentelemetry-go-contrib/instrumentation/net/http/otelhttp/internal/semconv/server.go

Lines 55 to 61 in c4223d5

	server.requestDurationHistogram, err = httpconv.NewServerRequestDuration(
	meter,
	metric.WithExplicitBucketBoundaries(
	0.005, 0.01, 0.025, 0.05, 0.075, 0.1,
	0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10,
	),
	)

opentelemetry-go-contrib/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/client.go

Lines 38 to 41 in c4223d5

	client.requestDuration, err = httpconv.NewClientRequestDuration(
	meter,
	metric.WithExplicitBucketBoundaries(0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10),
	)

opentelemetry-go-contrib/instrumentation/net/http/httptrace/otelhttptrace/internal/semconv/server.go

Lines 55 to 61 in c4223d5

	server.requestDurationHistogram, err = httpconv.NewServerRequestDuration(
	meter,
	metric.WithExplicitBucketBoundaries(
	0.005, 0.01, 0.025, 0.05, 0.075, 0.1,
	0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10,
	),
	)

This means that any long running transactions won´t show up correctly in the metrics.

Proposed Solution

I propose to either remove these hardcoded boundaries or make them configurable through a .WithBucketBoundaries(bounds ...float64)

Alternatives

No response

Prior Art

No response

Additional Context

No response

[dmathieu]

We can't remove the hardcoded boundaries, as that would make us non-compliant with the semantic conventions.
https://github.com/open-telemetry/semantic-conventions/blob/main/docs/http/http-metrics.md#metric-httpserverrequestduration

I don't know about the explicit option, as each metric has its own boundary. So that would mean one option per metric, and exposing each metric within the public API. But we could find a better design.

[ngehrsitz]

Where do you see the issue with e.g. otelhttp.NewHandler(h, "net/http", otelhttp.WithBucketBoundaries(1,2,10,30))?
From my search the duration metric is the only place with explicit boundaries: https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-go-contrib%20WithExplicitBucketBoundaries&type=code
What is preventing us from just passing that option trough?

[dmathieu]

Even though today, there's only one metric with bucket boundaries, there could be more tomorrow.
Also, from an outside point of view, WithBucketBoundaries is not meaningful at all. Bucket boundaries of what?

[ngehrsitz]

If we make the options configurable like this:

func NewHTTPClient(meter metric.Meter, metricOptions ...metric.Float64HistogramOption) HTTPClient {
	client := HTTPClient{}

	var err error
	client.requestBodySize, err = httpconv.NewClientRequestBodySize(meter)
	handleErr(err)

	client.requestDuration, err = httpconv.NewClientRequestDuration(
		meter,
		append([]metric.Float64HistogramOption{
			metric.WithExplicitBucketBoundaries(0.005, 0.01, 0.025, 0.05, 0.075, 0.1, 0.25, 0.5, 0.75, 1, 2.5, 5, 7.5, 10),
		}, metricOptions...)...,
	)
	handleErr(err)

	return client
}

then we are future-proof and could configure it on the top level as otelhttp.NewTransport(nil, otelhttp.WithDurationMetricBuckets(1)).
The alternative to that would be to have generic otelhttp.WithHistogramOptions(metric.WithExplicitBucketBoundaries(1)) but that would be less specific.