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

WIP: add telemetry #512

Open
wants to merge 3 commits into
base: dev/telemetry
Choose a base branch
from

Conversation

spencerdcarlson
Copy link
Contributor

No description provided.

]},
{erl_opts, [warnings_as_errors, {d, build_brod_cli}]}
{erl_opts, [warnings_as_errors, {d, build_brod_cli}, {d, brod_use_telemetry}]}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm convinced this is not necessary.
Please remove it.

@zmstone
Copy link
Contributor

zmstone commented Jul 8, 2022

Thanks for the PR.

BTW. I originally wanted a solution without {telemetry, "~> 1.0"} in the dependency (and the brod_use_telemetry to be undefined by default).
So to push the responsibility to the wrapping project.

@zmstone zmstone changed the base branch from master to dev/telemetry July 8, 2022 19:44
@zmstone
Copy link
Contributor

zmstone commented Jul 8, 2022

I have changed the base to dev/telemetry
let's start instrumenting the code, preferably increment with small PRs.

@maxno-kivra
Copy link

This would be very nice to have. What is needed to get this in?

@akosasante
Copy link

👋  Also looking to help push this forward, @zmstone , how can I help?

@mikpe
Copy link
Contributor

mikpe commented Nov 25, 2023

Use dependency injection.
Have measurement points invoke a local, "virtual", telemetry module.
That module checks if a telemetry callback module has been registered in the environment: if it has, pass the telemetry on to it, if not, do nothing.
The end result is that this project has no hard-coded dependencies in any telemetry framework, but the user can hook one up via the callback module they control.

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

Successfully merging this pull request may close these issues.

5 participants