Allow multiple views per instrument

[jsuereth]

• Allow multiple views per instrument
• Log on view conflict instead of throwing an exception.
• Update error reporting (slightly)

This does NOT

• Attempt to only poll async metrics once-per-instrument (instead once-per-metric for now)
• Give the best error messages

[jkwatson]

one little gripe & a corresponding nit, but this looks pretty solid to me.

[jack-berg]

#nit: This message isn't expensive to compute so let's omit the supplier. Same applies to the other catch log messages in this class.

Suggested change

	logger.log(Level.WARNING, e, () -> "Failed to register metric.");
	logger.log(Level.WARNING, "Failed to register metric.", e);

[jkwatson]

@jsuereth something for a follow-on PR

[jsuereth]

Good point, the expensive calculation is in a downstream branch. Once I fix the error messages for better provenance we can account for this.

[jack-berg]

The only remaining usages of the singular version of this method above (public View findView(InstrumentDescriptor descriptor, InstrumentationLibraryInfo meter)) are in tests, so I believe it can be removed.

[jsuereth]

Great catch, will remove in next cleanup CL. Thought I had removed it in this branch, but apparently missed it.

[jack-berg]

These register methods are fairly repetitive. Could DRY it up with something like:

  public final WriteableMetricStorage registerSynchronousMetricStorage(
      InstrumentDescriptor instrument, MeterProviderSharedState meterProviderSharedState) {
    List<SynchronousMetricStorage<?>> storage =
        register(
            instrument,
            meterProviderSharedState,
            view ->
                SynchronousMetricStorage.create(
                    view,
                    instrument,
                    meterProviderSharedState.getResource(),
                    getInstrumentationLibraryInfo(),
                    meterProviderSharedState.getStartEpochNanos()));
    if (storage.size() == 1) {
      return storage.get(0);
    }
    // If the size is 0, we return an, effectively, no-op writer.
    return new MultiWritableMetricStorage(storage);
  }

  public final void registerLongAsynchronousInstrument(
      InstrumentDescriptor instrument,
      MeterProviderSharedState meterProviderSharedState,
      Consumer<ObservableLongMeasurement> metricUpdater) {
    register(
        instrument,
        meterProviderSharedState,
        view ->
            AsynchronousMetricStorage.longAsynchronousAccumulator(
                view,
                instrument,
                meterProviderSharedState.getResource(),
                getInstrumentationLibraryInfo(),
                meterProviderSharedState.getStartEpochNanos(),
                metricUpdater));
  }
  ..
  private <T extends MetricStorage> List<T> register(
      InstrumentDescriptor instrument,
      MeterProviderSharedState meterProviderSharedState,
      Function<View, T> storageFunction) {
    List<View> views =
        meterProviderSharedState
            .getViewRegistry()
            .findViews(instrument, getInstrumentationLibraryInfo());
    List<T> storage = new ArrayList<>(views.size());
    for (View view : views) {
      try {
        storage.add(getMetricStorageRegistry().register(storageFunction.apply(view)));
      } catch (DuplicateMetricStorageException e) {
        logger.log(Level.WARNING, e, () -> "Failed to register metric.");
      }
    }
    return storage;
  }

If you want to do this, you'd also have to change the constructor of MultiWritableMetricsStorage:

MultiWritableMetricStorage(List<? extends WriteableMetricStorage> metrics)

[jsuereth]

There's cleanup/fixes coming here in future PRs. TL;DR; we likely need a different registration mechanism for sync vs. async. Happy to discuss the details if you're interested, but expect these to diverge when we fix the issue that async instruments collect multiple times per collection period.