Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Library to support Telemetry.Metrics as OpenTelemetry metrics #303

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tsloughter
Copy link
Member

Not ready because still using local changes in the deps but wanted to open it up for review while those changes are being merged and released.

@tsloughter
Copy link
Member Author

Forgot I needed to make meter a per-metric option. Though that isn't actually going to work since the main purpose of this is supporting stuff like Phoenix which isn't going to add meter to the reporter opts of telemetry events...

So may just add a warning in the docs that Scope is going to be wrong for all your Telemetry.Metrics

Copy link

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

require Logger
use GenServer

@doc """

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@doc """
@doc """
Starts open telemetry metrics reporter process.
Accepts the same options as `GenServer.start_link/2`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last line seems wrong? Or at least confusing. That'd make me think the options are ones for configuring the GenServer, stuff like hibernate, but its the options to be passed to init.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right, sorry.

…etrics.ex

Co-authored-by: José Valim <jose.valim@gmail.com>
@AndrewDryga
Copy link

Can I help you guys somehow? Have to submit metrics to Google Metrics for alerting and, since our metrics are not mission critical, we can try to pilot this PR in production.

@tsloughter
Copy link
Member Author

@AndrewDryga yes, definitely! Are you on slack? I hit a bug in metrics that has kept me from having time to spend on docs but could help you on slack to get going.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants