Propose histogram bucket boundary metric advice (aka hint API)

[jack-berg]

Fixes #2229.
Related to #3061 (lays groundwork but does not resolve).
Related to #2977, which may use this new API to have http.server.duration report in seconds instead of ms without changing / breaking default bucket boundaries.

Summary of the change:

• Proposes a new parameter to optionally include when creating instruments, called "advice".
• For the moment, advice only has one parameter for specifying the bucket boundaries of explicit bucket histogram.
• Advice can be expanded with additional parameters in the future (e.g. default attributes to retain). The parameters may be general (aka applicable to all instruments) or specific to a particular instrument kind, like bucket boundaries.
• Advice parameters can influence the default aggregation, which is used if there is no matching view and if the reader does not specify a preferred aggregation.
• Not clear that all advice will be oriented towards configuring aggregation, so I've intentionally left the scope of what they can influence open ended.

I've prototyped this in java here. Example usage:

DoubleHistogram doubleHistogram =
        meterProvider
            .get("meter")
            .histogramBuilder("histogram")
            .setUnit("foo")
            .setDescription("bar")
            .setAdvice(
                advice -> advice.setBoundaries(Arrays.asList(10.0, 20.0, 30.0)))
            .build();

Advice could easily be changed to "hint" with everything else being equal. I thought "advice" clearly described what we're trying to accomplish, which is advice / recommend the implementation in providing useful output with minimal configuration.

[weyert]

I really like this idea.

I am wondering if we want to enhance this so you can also define the aggregation of a metric?

[jsuereth]

Awesome small and targeted proposal! I think we (additionally) need the two features I mentioned as part of stabilizing HTTP semconv, but happy to have them as follow on features.

[jack-berg]

I've marked the PR as ready for review. I think there are additional advice arguments we can expose (I'm most excited about solving #3061), but that this is a great start and probably something we can all agree on.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[jack-berg]

I've pushed a commit that adjusts the document status to be "Stable except where otherwise specified", and calls out the new advice portions as experimental.

This has been open for ~6 weeks and the feedback has mostly positive with some requests for additional advice parameters that could be added in separate PRs.

@open-telemetry/specs-metrics-approvers please review so we can reach a resolution.

[jianwu]

I like this change that could unblock our adoption of Otel. In all our existing use case (on OC), user will always provide buckets for any histogram. However as bucket is not a secondary citizen when defining a histogram. I'm wondering why we don't make it as explicit option parameters for histogram just like what Prometheus Java Client does.
That will make the API more self explained and easy to use. Advice sounds more like a hint, indicates something could be ignored most of the time unless you really need it.

[jack-berg]

The OpenTelemetry SDK is highly configurable in terms of how measurements are aggregated and exported from an application. Whether the bucket boundaries were to be provided as a parameter for the histogram instrument or via this advice API, the SDK operator would always have the ability to override it. The value of ExplicitBucketBounds advice in this PR would actually end up getting used in many situations since it influences the default aggregation which I would guess is used the majority of the time. A user operating the SDK would have to go out of their way with views or MetricReader configuration to ignore the advice.

[jack-berg]

@pirgeo, @reyang, @cijothomas, @arminru Tagging the others who have previously commented, please take another look. Would be good to get this in so we can start gathering feedback.

[reyang]

LGTM. A non-blocking thing for us to think about - do we want the "advice" to be a flexible thing such as a single JSON string? This would allow vendors to add their specific hints rather than requiring an API change. It also allows the API/SDK to move faster without having to bundle the release cycles (e.g. think about this, if we're going to add another "advice" regarding "recommended dimensions", with the strongly typed approach it'll probably take 6-12 months).

[weyert]

LGTM. A non-blocking thing for us to think about - do we want the "advice" to be a flexible thing such as a single JSON string? This would allow vendors to add their specific hints rather than requiring an API change. It also allows the API/SDK to move faster without having to bundle the release cycles (e.g. think about this, if we're going to add another "advice" regarding "recommended dimensions", with the strongly typed approach it'll probably take 6-12 months).

I think @jmacd might have suggested something similar in the past?

[pirgeo]

LGTM too. About flexible advice - I am not sure I understand how and where this would be parsed so that it is flexible enough for all (or at least most) needs - @reyang could you elaborate on that a bit more? Seems like a big chunk of work to me, and might be outside the scope of this PR.

[jack-berg]

A non-blocking thing for us to think about - do we want the "advice" to be a flexible thing such as a single JSON string?

I think its worth exploring (in a subsequent iteration 🙂). It would essentially allow us to have an API that remains stable, while incrementally adding to the SDK specification.

The API might accept a JSON string, or some other generic structure like a map:

meter.histogramBuilder("histogram")
   .setUnit("foo")
   .setDescription("bar")
   .setAdvice(Map.of("explicitBucketBoundaries", List.of(10.0, 20.0, 30.0));

And its up to the SDK to interpret the advice and extract out any relevant bits. In this case, the SDK would look for "explicitBucketBoundaries" entry in the map, and interpret the value as the bucket boundaries.

Later, if we add additional advice for default attribute keys, there's no need for the API to change:

meter.histogramBuilder("histogram")
   .setUnit("foo")
   .setDescription("bar")
   .setAdvice(Map.of(
      "explicitBucketBoundaries", List.of(10.0, 20.0, 30.0),
      "defaultAttributeKeys", List.of("a", "b", "c")
    );

The SDK just needs to add additional language to interpret the new "defaultAttributeKeys" entry in the map.