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 the ability to filter the metrics to expose #10

Open
dkurzaj opened this issue Feb 13, 2019 · 7 comments
Open

Add the ability to filter the metrics to expose #10

dkurzaj opened this issue Feb 13, 2019 · 7 comments

Comments

@dkurzaj
Copy link
Contributor

dkurzaj commented Feb 13, 2019

Vespa exposes a lot of metrics, and this exporter collects them from all the nodes of a cluster. So when it exposes them it can generate a big quantity of data (I'm experiencing a size of 65Mo). It is not very suitable since the Prometheus server will thus take a lot of time to load the data exposed by this exporter. And I only use a few of those metrics.

A simple solution would be to give the ability to add a configuration file to this exporter with the names (in Prometheus format) of the metrics it has to expose. So that the users of the exporter could choose to expose only the metrics they need.

What do you think about that ?

I would gladly implement a solution to this issue.

@bratseth
Copy link
Member

bratseth commented Feb 14, 2019

Sounds good to me. We do this for our internal metrics integration as well, so there is already syntax for it in services.xml:

<admin>            
    <metrics>
        <consumer id='e.g promotheus'>
            <metric id='some.metric'/>
            ...
        </consumer>
    </metrics>
<admin>            

I suggest that we

  • Document this syntax in https://docs.vespa.ai/documentation/reference/services-admin.html
  • Create a new config definition in Vespa which contains this information only (currently we only have it as part of a larger, internal config for our internal metrics integration).
  • Expose that new config from the config model.
  • Look it up from the Prometheus exporter using the config/v2 REST API of the config server to do what you need.

Gjøran, perhaps you could help with this?

@bratseth
Copy link
Member

Btw, do you want to be a committer to this repo dkurzaj? I think that makes sense.

@dkurzaj
Copy link
Contributor Author

dkurzaj commented Feb 14, 2019

That seems like a nice integration indeed!

Thank you, I would gladly become a committer. :)

In the mean time, to fit my needs, I have already implemented a first naive solution which reads a file containing the whitelisted metrics (one per line) and only expose them. Would you like me to make a PR with it to temporarily fix this issue or do you prefer that we wait for the changes you described ?

@bratseth
Copy link
Member

Thanks - I sent you an invite.

We try to avoid depending on local files so maybe it is better to wait for the config-backed solution, if you have a little bit of time to work on it.

@dkurzaj
Copy link
Contributor Author

dkurzaj commented Feb 14, 2019

Understood!

@dkurzaj
Copy link
Contributor Author

dkurzaj commented Oct 10, 2019

I did not really follow the new features of Vespa, but by reading the latest blog post (https://blog.vespa.ai/vespa-product-updates-september-2019-tensor/) it seems that this project will not be needed to monitor through Prometheus for the future versions of Vespa is it right?

@gjoranv
Copy link
Member

gjoranv commented Oct 10, 2019

I hope not. If there's something missing from the new integration, we should rather add it there. It mainly consists of two classes:
PrometheusHandler
and
PrometheusUtil

It's based on a PR from an external contributor. I haven't used Prometheus myself, so it would be great if you want to try it out.

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

No branches or pull requests

3 participants