opentelemetry-erlang
opentelemetry-erlang copied to clipboard
Add prometheus exporter
First draft of the Prometheus exporter to collect some feedback.
Open points:
-
At the moment it is embedded in the
opentelemetry_experimental
app to speed up development, should it be a separate app? -
As suggested in the spec I used a reader and an exporter but from the user point of view the exporter is masked and its config parameters are on the reader, for example:
config :opentelemetry_experimental, readers: [ %{ module: :otel_metric_reader_prometheus, config: %{ add_scope_info: true, add_total_suffix: true, endpoint_port: 3000 } } ]
default_temporality_mapping
,export_interval_ms
andexporter
options on the reader are overridden, should we emit a warning if the user configures them? -
The encoding part works but it can be improved for sure, I still need to go through that module again and refactor it. Anyway, any suggestion is welcomed.
-
The
otel_metric_reader_prometheus
is in reality a supervisor that starts the reader itself and the httpd server if needed. The httpd server is started only if anendpoint_port
is given to the configuration. We can maybe have a more explicit configuration field for this? -
If the httpd server is not used it should be possible for the user to get the metrics in prometheus format so that he can expose them. This can be done by calling the
otel_metric_reader_prometheus:collect/1
function but it needs the reader pid as an argument, not so user-friendly. -
How to implement the same behavior of
erlang:float_to_binary(2.0, [short])
that was added in OTP 25? (tests failing for this reason)
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 67.98%. Comparing base (
8f01a9a
) to head (af3826b
). Report is 3 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #619 +/- ##
==========================================
- Coverage 74.48% 67.98% -6.51%
==========================================
Files 56 61 +5
Lines 1862 2152 +290
==========================================
+ Hits 1387 1463 +76
- Misses 475 689 +214
Flag | Coverage Δ | |
---|---|---|
api | 58.84% <ø> (-14.67%) |
:arrow_down: |
elixir | 17.32% <ø> (?) |
|
erlang | 74.35% <ø> (-0.14%) |
:arrow_down: |
exporter | 67.47% <ø> (ø) |
|
sdk | 78.69% <ø> (-0.01%) |
:arrow_down: |
zipkin | 54.16% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I'm very interested in use case 5, since we already have our own web server and do not want to export an additional port.
I'm very interested in use case 5, since we already have our own web server and do not want to export an additional port.
Yea, I think this will be the most common use case, we need to find a way to make this easy for users
I am also interested in use case 5. Will evaluate switching to otel collector but for now this would go in nicely on my current project 😄
@albertored are you able to update this PR? I'd like to get prometheus support in so we can get the technical committee to sign off on our metrics implementation so we can go GA.
@tsloughter I'll try this weekend hoping things have not changed too much. Unfortunately in the last months I was not able to follow the project with constancy
@albertored I've rebased and modified your version a bit https://github.com/open-telemetry/opentelemetry-erlang/compare/main...travelping:opentelemetry-erlang:feature/prometheus-exporter-for-upstream
The reader logic can now be used from a cowboy handler as well (sample included).
@tsloughter rebased and pushed with additions from @RoadRunnr, we should still resolve the points in my first comment before merging
PS: this commit is causing failures in the pipeline since maps:iterator/2
was introduced in erlang 26. I don't know if there is an efficient way of doing it without that function (I can only think about converting map to list and order it before folding). If not we should revert that commit
PS: this commit is causing failures in the pipeline since
maps:iterator/2
was introduced in erlang 26. I don't know if there is an efficient way of doing it without that function (I can only think about converting map to list and order it before folding). If not we should revert that commit
The ordering is only used for the tests to get defined ordering for comparing the results. It is therefore not to critical that they are super efficient. The simple sort of the map keys would be enough. I can supply a patch if you want?
we should still resolve the points in my first comment before merging
point 5 (or use case 5) should be solved. A sample cowboy handler is included.
The encoding part works but it can be improved for sure.
I do have additional changes pending in https://github.com/travelping/opentelemetry-erlang/compare/bbcfe2f6370c89fcd8c7ed5981fd8c171aeff4d0...0d5f677ccc64a069274689aeeee70663acb41eed