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

Move Exporter to /collector so it can be used as a library #209

Merged
merged 1 commit into from
Aug 26, 2017

Conversation

arvenil
Copy link
Contributor

@arvenil arvenil commented Jun 15, 2017

This decouples collector implementation from http server and cli client.
This way I can create my own binary with several collectors by using collector.New.

All scrappers were already in /collector subpackage, so I just moved Exporter struct which is implementation of prometheus Collector, also to /collector subpackage.

@SuperQ
Copy link
Member

SuperQ commented Jun 15, 2017

Great idea!

@arvenil
Copy link
Contributor Author

arvenil commented Jun 15, 2017

Builds now, I can rebase if you wish.

@brian-brazil
Copy link
Contributor

This way I can create my own binary with several collectors by using collector.New.

This is not a recommended way to deploy things, see https://prometheus.io/docs/instrumenting/writing_exporters/#deployment

@arvenil
Copy link
Contributor Author

arvenil commented Jun 15, 2017

@brian-brazil I know that. It doesn't change anything for prometheus exporters.

It lets me to implement my own http server, with my own modifications... which I will likely try to push upstream if they make sense for you guys, but there might be some that doesn't.

This keeps cli and http interface in one package, and collector in another one - making easier to develop things on top of the collector.

Exporter is implementation of prometheus Collector interface which I can't access because it's in main package. It should also make unit testing easier.

@SuperQ
Copy link
Member

SuperQ commented Jul 5, 2017

Looks like this needs a rebase.

@arvenil arvenil force-pushed the exporter_is_collector branch 2 times, most recently from a3e396f to 2afc97b Compare August 7, 2017 11:39
@arvenil arvenil force-pushed the exporter_is_collector branch from 2afc97b to 5ad6fbd Compare August 7, 2017 11:43
@arvenil
Copy link
Contributor Author

arvenil commented Aug 7, 2017

Done.

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

@SuperQ
Copy link
Member

SuperQ commented Aug 10, 2017

@grobie PTAL

Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

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

LGTM

@SuperQ SuperQ merged commit a3a0fc3 into prometheus:master Aug 26, 2017
@frimik
Copy link

frimik commented Nov 11, 2017

nevermind, my bad!

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