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

Add environment variable for config.stats.interval #1627

Closed
rg2011 opened this issue Jul 26, 2024 · 6 comments
Closed

Add environment variable for config.stats.interval #1627

rg2011 opened this issue Jul 26, 2024 · 6 comments

Comments

@rg2011
Copy link
Contributor

rg2011 commented Jul 26, 2024

Component

Feature enhancement

Is your feature request related to a problem? Please describe

Stat collection would be easier to manage in dockerized environment if config.stats.interval could be overriden by an environment variable, as is the case for many other config settings.

Describe the solution you'd like

Add an IOTA_STATS_INTERVAL environment variable that overrides the value of config.stats.interval

Describe alternatives you've considered

Replace the whole config.js file with a kubernetes configmap, but that would make maintenance harder, if we have to track and replicate in our configmap any changes to the builtin config.js when a new version is released.

The systemd version you checked that didn't have the feature you are asking for

4.5.0

@fgalan
Copy link
Member

fgalan commented Jul 29, 2024

PR #1628

@rg2011
Copy link
Contributor Author

rg2011 commented Aug 1, 2024

I have been testing the changes in PR #1628 and they work, but I find the push model for statistics too cumbersome:

  • The kpis mongo collection has to be manually created as capped, or rotated periodically, otherwise it might grow unbounded.
  • To expose those metrics to telemetry systems such as Prometheus, an additional exporter has to be developed.

Besides, comments in PR #1628 suggest that these push stats have actually never been used.

All above considered, I am tempted to suggest deprecating the push mechanism altogether, and replacing it with an openmetrics endpoint in the northbound API.

  • Exposing the stats in openmetrics format would allow direct integration with Prometheus and any other compatible telemetry tool.
  • The app would not need to bother with global and local stats, nor timers to periodically push and clear them. Pull based telemetry tools manage the collection intervals themselves, and calculate the increments and rates between consecutive measures.

I might work on such an endpoint in another PR.

@fgalan
Copy link
Member

fgalan commented Aug 5, 2024

PR #1629

AlvaroVega added a commit that referenced this issue Aug 7, 2024
@fgalan
Copy link
Member

fgalan commented Aug 7, 2024

Now that PR #1629 has been merged, can this issue be closed?

@rg2011
Copy link
Contributor Author

rg2011 commented Aug 8, 2024

I tested the :latest image in our kubernetes environment and found out that the prometheus version available there actually requests openmetrics version 0.0.1, so I had to add support for that version too. I submitted PR #1640

@fgalan
Copy link
Member

fgalan commented Sep 18, 2024

Having releases version 4.6.0 including this issue, I think is time to close the issue. @rg2011 tell us otherwise, so we can reopen it.

@fgalan fgalan closed this as completed Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants