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

feature: introduce Prometheus (#976) #2095

Closed
wants to merge 1 commit into from

Conversation

blackheaven
Copy link

No description provided.

@slingamn
Copy link
Member

Thanks for the PR! Could you clarify the goals here? The increase in binary size (from 14 MB to 18 MB) and the large number of additional third-party dependencies pose potential difficulties.

@blackheaven
Copy link
Author

I'm not really at ease with go.

I have followed the guide and it changed vendors, but I'm not sure I should have added the to git.

@slingamn
Copy link
Member

Committing vendor/ to git is the correct convention in this specific repository :-)

My question is, what goals does this change serve? Are you, specifically, seeking to monitor a particular application health metric? Is Prometheus support just something you feel that Ergo should have?

My gutfeeling is that either people should use something like ncabatoff/process-exporter to monitor Ergo's health from the outside, or we should carefully evaluate the use cases for monitoring and build something that satisfies them (which may not involve bringing Prometheus inside the application core).

@blackheaven
Copy link
Author

Oh, I see.

I have just provided a first implementation following your issue.

My opinion is that in the middle-term it would be great to have more business-center metrics such as:

  • Registered user count
  • Online user count
  • Number of channels
  • Number of buffered messages
  • etc.

If I might ask, what do you have in mind at the time you have created this issue?

@blackheaven blackheaven changed the title feature: introduce Promethes (#976) feature: introduce Prometheus (#976) Nov 1, 2023
@slingamn
Copy link
Member

slingamn commented Nov 7, 2023

Sorry, I completely forgot that I had an open issue for this :-)

I think this is exactly what I had in mind when I wrote up the issue originally, but given how heavyweight it turned out to be in terms of dependencies and binary size, I'm not sure I want to go ahead with it. I'm very appreciative of your interest in the project, though! Let me know if you have needs as an Ergo operator (monitoring or otherwise) that I can work on.

@blackheaven
Copy link
Author

No problem, I don't have this need for the moment.

@blackheaven blackheaven closed this Nov 7, 2023
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.

None yet

2 participants