Log an error for empty view criteria

[MrAlias]

The specification recommends:

• If no criteria is provided, the SDK SHOULD treat it as an error. It is recommended that the SDK implementations fail fast. Please refer to Error handling in OpenTelemetry for the general guidance.

This is not treated as an error:

opentelemetry-go/sdk/metric/view.go

Lines 57 to 59 in b2246d5

	if criteria.empty() {
	return emptyView
	}

An error should be logged to match the other existing error already logged by NewView:

opentelemetry-go/sdk/metric/view.go

Lines 94 to 98 in b2246d5

	global.Error(
	err, "not using aggregation with view",
	"criteria", criteria,
	"mask", mask,
	)

Originally posted by @MrAlias in #3645 (comment)

[pellared]

An error should be logged

Why not returning an error as suggested per the specification? I would consider changing the declaration to:

func NewView(criteria Instrument, mask Stream) (View, error) {

1. We still can return a view when a error is reported
2. I guess usually the operators (users) do not want such errors and would prefer to react. In some cases they may even want to crash the app if they find that they do not accept a misconfigured View.

[MrAlias]

Returning an error means NewView will not be able to be passed directly to WithView.

[MrAlias]

Returning an error means NewView will not be able to be passed directly to WithView.

That is the reason we already log an error with the global logger. It makes sense to keep the pattern here.

[pellared]

On the other hand, the exporter factory functions like otlpmetrichttp.New are returning (metric.Exporter, error) so that one cannot directly pass the result to metric.NewPeriodicReader.

I see Go as a quite verbose language when it comes to error handling. I do not feel that "fluent-like" API is idiomatic. The user can always create a helper that would handle the error (e.g. log, ignore, panic).

[MrAlias]

I don't follow. You are suggesting that precedence from a different function from a different package from a different part of the setup should take precedence for NewView? A function with existing precedence for error handling?

[pellared]

That is only "one" reason out of three.

1. I feel that explicit error handling is more idiomatic
2. It would literally follow the OTel Specification recommendation
3. Precedence from a different package

(in this order)