opentelemetry-erlang icon indicating copy to clipboard operation
opentelemetry-erlang copied to clipboard

MetricReader Supervisor

Open tsloughter opened this issue 2 years ago • 3 comments

A MeterProvider (otel_meter_server) can have many MetricReaders (otel_metric_reader). Right now they are started in the MeterProvider with start_monitor`. Instead there should likely be a MetricReader supervisor.

The meter reader is using start_monitor because the original idea was readers could continue operating without the meter server, but this isn't the case right now since the ets tables are owned by the meter server. Unless this is changed the monitor needs to be switched to a link. Also need to be able to recover who the readers are when the meter server restarts.

Discovery by looking at a supervision tree could be a good idea as it would allow for added readers to be returned to the server after a crash.

tsloughter avatar Aug 11 '22 12:08 tsloughter

I think the path forward will be requiring Readers configured at startup. There will be no add_metric_reader. The metrics supervisor will start the otel_meter_server and a supervisor of metric reader workers.

I haven't decided the best way to get the readers in the server, I'm guessing just asking the reader supervisor for its children is best. Since it won't be a simple_one_for_one it'll always have the full list of readers.

Only question then is if I should generate an atom name for each reader to use by the server or monitor the readers from the server.

And I guess the sup can be a one-for-one since the server crashing won't mean the readers need to restart -- I think.

tsloughter avatar Aug 13 '22 11:08 tsloughter

Argh, but need to figure out how to share the ETS table that is per-reader between the reader and the server.

tsloughter avatar Aug 13 '22 16:08 tsloughter

Actually, maybe it should be reverse. The Reader will look up the server in the supervisor and then tell it about itself and its tables.

tsloughter avatar Aug 13 '22 16:08 tsloughter