Make opentelemetry_metrics_exporter entrypoint support pull exporters

[aabmass]

The opentelemetry_metrics_exporter is currently the option for configuring metric exporters for auto-instrumentation. Unfortunately, this doesn't work for pull exporters (e.g. prometheus) which are implemented as MetricReader subclasses, nor does it allow the entrypoint impl to set configure the MetricReader. Solution was discussed in #2843 (comment)

If I could make a suggestion, the entrypoints for OTEL_METRIC_EXPORTERS should provide a MetricReader factory function. For example, for otlp that could be something like

def provide_reader() -> MetricReader:
  preferred_temporality=environ.get(
                    OTEL_EXPORTER_OTLP_METRICS_TEMPORALITY_PREFERENCE,
                    "CUMULATIVE",
                )
                .upper()
                .strip()

  return PeriodicExportingMetricReader(
    exporter=OTLPExporter(),
    preferred_temporality=preferred_temporality,
    ...,
  )

then update

opentelemetry-python/exporter/opentelemetry-exporter-otlp-proto-grpc/setup.cfg

Line 62 in b9a6358

otlp_proto_grpc = opentelemetry.exporter.otlp.proto.grpc.metric_exporter:OTLPMetricExporter

to

 otlp_proto_grpc = opentelemetry.exporter.otlp.proto.grpc.metric_exporter:provide_reader

one downside of this approach is some code duplication across implementations of provide_reader() for other exporters if we have common environment variables to check. However it would simplify things for using MetricReaders like Prometheus which act as a "pull exporter", as they can easily implement this method as well. Right now im not sure how we would implement OTEL_METRICS_EXPORTER=prometheus

and I think we are all on board with this approach (see #2864 (comment)).

[aabmass]

Recap of the discussion from the SIG:

1. This is a breaking change, e.g. Azure implements this entrypoint and would have to modify it
2. It's inconsistent with Logging and Tracing where the entrypoints are expected to return an exporter and not a Span/Log processor
3. It's probably an oversight that the logging and tracing entrypoints are implemented to return exporters instead of processors

Possible paths forward:

1. Go ahead with the approach outline above, implemented in Update opentelemetry_metrics_exporter entrypoint to a MetricReader factory #3412 and leave the tracing/logging entrypoints inconsistent
2. Have two entrypoints for each signal, one for the exporter and one for the processor. Implementors of an exporter may provide one of those two entrypoints
   • pros: most flexible, no breaking changes, can do the work incrementally
   • cons: adds complexity (6 entrypoints instead of 3), more confusing for implementors of the entrypoint
3. Use the existing entrypoints but lossen them to allow either returning an exporter or a processor/reader Callable[[], MetricReader] | Callable[[], MetricExporter], Callable[[], SpanProcessor] | Callable[[], SpanExporter], etc.. In the _init_metrics() etc. methods, we can handle the two cases
   • basically the same pros and cons as 2. but without adding more entrypoints. May be simpler for implementors.

Please add your thoughts to move this forward.

[aabmass]

I think we should also look at the config proposed by Configuration WG https://github.com/open-telemetry/opentelemetry-configuration and make sure it aligns with what we do.

[pmcollins]

I'd vote for exploring option 2 (did you mean reader instead of processor?) -- it's backwards compatible, is type safe/consistent, solves the problem for Prometheus users, and appears to be only a (minor) inconvenience for authors of pull-based exporters (of which, to my knowledge, there is only one).

[aabmass]

Here's an example snippet from OTel configuration yaml as proposed:

meter_provider:
  # Configure metric readers.
  readers:
    # Configure a pull-based metric reader.
    - pull:
        exporter:
          # Configure exporter to be prometheus.
          prometheus:
            # ...
    # Configure a periodic metric reader.
    - periodic:
        interval: 5000
        timeout: 30000
        exporter:
          otlp:
            # ...

tracer_provider:
  # Configure span processors.
  processors:
    - batch:
        schedule_delay: 5000
        export_timeout: 30000
        exporter:
          otlp:
            # ...

This is more hierarchical than what we current support with opentelemetry-instrument, I think we will need to make pretty invasive changes to our existing entrypoints when we implement this. It's probably best to just do a temporary workaround then to make a two sets of big breaking changes.

[aabmass]

Discussed in SIG today. Decided because our current entrypoint structure is so different from what the Configuration WG has proposed, there will probably be breaking changes. Until then, we should do a minimal workaround to avoid churn but still support Prometheus.

I will send a PR that instead just checks that if the entrypoint points to a MetricReader it will use that directly. The existing behavior of using PeriodicExportingMetricReader with a push exporter will remain the same. This is basically option 3 above but only scoping it to metrics for now.