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

lf-monitor: init LF metrics collector #53

Draft
wants to merge 1 commit into
base: open-source
Choose a base branch
from

Conversation

fstreun
Copy link
Collaborator

@fstreun fstreun commented Sep 17, 2024

This change is Reviewable

Copy link
Member

@marcfrei marcfrei left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion / 0 of 1 LGTMs obtained / 0 of 1 approvals obtained

a discussion (no related file):
Thanks a lot, Fabio. Didn't look too closely yet at the changeset, but I'm a little hesitant to introduce a dependency on Rust unless it clearly seems valuable. What's the reasoning behind this?


Copy link
Collaborator Author

@fstreun fstreun left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion / 0 of 1 LGTMs obtained / 0 of 1 approvals obtained

a discussion (no related file):

Previously, marcfrei (Marc Frei) wrote…

Thanks a lot, Fabio. Didn't look too closely yet at the changeset, but I'm a little hesitant to introduce a dependency on Rust unless it clearly seems valuable. What's the reasoning behind this?

Hi Marc

Description is still missing. But here a short summary.
The overall goal is to enhance the monitoring experience by providing a Prometheus endpoint that expose all of the metrics in a reasonable way. The currently described approach using telegraf is not optimal because the dependency is rather big and all LF metrics, including their parameters, must be defined in telegraf's config file. This is cumbersome and does not allow for metrics with a dynamic set of possible parameters.

IMO, we should have full control over the set of metrics that we collect and the way we export them. I.e., we should implement the translation from DPDK's telemetry interface to a Prometheus endpoint.

The choice of using Rust is rather opportunistic. I simply wanted to use Rust in a small project. I don't see an advantage of implementing this feature in Rust instead of Go, besides the general advantages of Rust (which also comes with its disadvantages).

I wouldn't mind moving this project in a separate repository.


Copy link
Member

@marcfrei marcfrei left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion / 0 of 1 LGTMs obtained / 0 of 1 approvals obtained

a discussion (no related file):

Previously, fstreun wrote…

Hi Marc

Description is still missing. But here a short summary.
The overall goal is to enhance the monitoring experience by providing a Prometheus endpoint that expose all of the metrics in a reasonable way. The currently described approach using telegraf is not optimal because the dependency is rather big and all LF metrics, including their parameters, must be defined in telegraf's config file. This is cumbersome and does not allow for metrics with a dynamic set of possible parameters.

IMO, we should have full control over the set of metrics that we collect and the way we export them. I.e., we should implement the translation from DPDK's telemetry interface to a Prometheus endpoint.

The choice of using Rust is rather opportunistic. I simply wanted to use Rust in a small project. I don't see an advantage of implementing this feature in Rust instead of Go, besides the general advantages of Rust (which also comes with its disadvantages).

I wouldn't mind moving this project in a separate repository.

I like the idea of taking full control over the metrics instead of relying on telegraf. And thanks for the explanation regarding the language choice. How about keeping lf-monitor here in tree as an experimental feature in Rust for the time being? We could then still decide whether we want to rewrite it in Go for improved maintainability later on.


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.

2 participants