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

Make Stackdriver main collector more library-friendly #157

Merged

Conversation

thepalbi
Copy link
Contributor

We would like to use stackdriver_exporter as a library. The project is almost usable as a library as it is, though inside the main collectors code (MonitoringCollector) some CLI flag are used from the global context.

This PR does a couple of changes to make MonitoringCollector more suitable for library use:

  • Moves all kingpin CLI flags in a same file flags.go. This cleans the package collectors, making it only content the actual collector's code.
  • Creates a new type MonitoringCollectorOptions, which will be used to send the newly created collector all required settings.
  • Moves the parsing of monitoring.metrics-type-prefixesand monitoring.filters to the main package. Also for cleaning up the collectors package.

thepalbi and others added 11 commits May 19, 2022 12:13
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Co-authored-by: kgeckhart <kgeckhart@users.noreply.github.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Co-authored-by: kgeckhart <kgeckhart@users.noreply.github.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Co-authored-by: kgeckhart <kgeckhart@users.noreply.github.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Co-authored-by: kgeckhart <kgeckhart@users.noreply.github.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
@thepalbi thepalbi force-pushed the pablo/make-library-friendly branch from 24ec49f to d24d5cd Compare May 19, 2022 15:13
Copy link
Contributor

@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.

Seems reasonable to me. Minor comment about the gitignore.

.gitignore Outdated Show resolved Hide resolved
@SuperQ
Copy link
Contributor

SuperQ commented May 19, 2022

We've talked about making more of our exporter collector packages interchangeable to allow more library use. We're trying to come up with a registration/config pattern for this.

If you're interested in contributing, I would love to hear your thoughts.

Signed-off-by: Pablo Balbi <pablo.l.balbi@gmail.com>
@thepalbi
Copy link
Contributor Author

We've talked about making more of our exporter collector packages interchangeable to allow more library use. We're trying to come up with a registration/config pattern for this.

If you're interested in contributing, I would love to hear your thoughts.

Definetely, feel free to tag me in any issue/thread were you are discussing. More than happy to contribute 😀

Copy link
Contributor

@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, Thanks!

@SuperQ
Copy link
Contributor

SuperQ commented May 19, 2022

I didn't find an issue for it, so I opened prometheus/exporter-toolkit#92.

@SuperQ SuperQ merged commit b114dab into prometheus-community:master May 19, 2022
SuperQ pushed a commit that referenced this pull request Nov 22, 2022
This PR introduces support for aggregating DELTA metrics in-memory. This implementation is somewhat based on #167 but with extra functionality which is intended to solve the problem identified here, #167 (comment).

I chose to inject interfaces for the delta and distribution stores at the Collector level to try to keep library friendliness intact, #157

I did as much as I could to explain the solution and the tradeoffs with aggregating deltas in this manner in the README.md. The bulk of the changes are in `monitoring_metrics.go` where I tried to obfuscate as much of the delta logic as possible.

Should hopefully resolve #116 and #25 

Signed-off-by: Kyle Eckhart <kgeckhart@users.noreply.github.com>
@kgeckhart kgeckhart mentioned this pull request Jan 25, 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