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

OpenTelemetry Collector Ecosystem Integration #370

Conversation

Nicolas-MacBeth
Copy link
Contributor

@Nicolas-MacBeth Nicolas-MacBeth commented Jun 26, 2020

This PR is meant to define the folder and introduce the community to the receiver I've been working on, and get feedback on it. It includes a README with the current state of the config which is also a really good in-depth config explanation.

Hi! I'm Nic, an intern @ Google.

prometheus_exec is a receiver meant to make it easy for users to get up-and-running quickly, collecting metrics from their favorite third-party services (that don't normally support instrumentation). This frictionless experience should help with adoption and be an attractive solution for those holding out on using the Collector because their systems/services were not compatible. In a nutshell, it runs binaries of Prometheus exporters (custom-built code that make third-party services, such as MySQL, Redis, etc. communicate the Prometheus protocol) and spawns the equivalent Prometheus receiver to collect the metrics. It handles crashing binaries by applying exponential backoff, can assign a random port and supports string templating. Please read the README (it really has a lot of useful info) or ask me for more details :) !

README link with markdown applied (much nicer read): link

For example, a user wants to quickly get metrics from their MySQL and Apache servers running on a VM. Simply, they indicate a couple of things in the configuration file, download the exporter binaries and run the Collector. It will handle starting the exporters, starting the equivalent receivers and making everything work like magic (hopefully).

Example configuration:

receivers:
    prometheus_exec/apache:
        exec: ./apache_exporter --retry=false
        port: 1234 
        scrape_interval: 60s

    prometheus_exec/postgresql:
        exec: ./postgresql_exporter --port {{port}} --config conf.xml
        port: 9876
        env:
          - name: DATA_SOURCE_NAME
            value: user:password@url:port/dbname

@Nicolas-MacBeth Nicolas-MacBeth requested a review from a team June 26, 2020 22:43
@codecov
Copy link

codecov bot commented Jun 26, 2020

Codecov Report

Merging #370 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #370   +/-   ##
=======================================
  Coverage   82.59%   82.59%           
=======================================
  Files         170      170           
  Lines        9063     9063           
=======================================
  Hits         7486     7486           
  Misses       1251     1251           
  Partials      326      326           
Flag Coverage Δ
#integration 0.00% <ø> (ø)
#unit 82.59% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fba66a...c6c6344. Read the comment docs.

Copy link
Member

@james-bebbington james-bebbington left a comment

Choose a reason for hiding this comment

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

Left a few comments


Side note:

README link with markdown applied (much nicer read): link

You can get to the raw file from the PR via:

image

receiver/prometheusexec/README.md Outdated Show resolved Hide resolved
receiver/prometheusexec/README.md Outdated Show resolved Hide resolved
receiver/prometheusexec/README.md Outdated Show resolved Hide resolved
receiver/prometheusexec/README.md Show resolved Hide resolved
receiver/prometheusexec/README.md Outdated Show resolved Hide resolved
@james-bebbington
Copy link
Member

@Nicolas-MacBeth Close this?

@bogdandrutu
Copy link
Member

@james-bebbington why closing? Is it no longer needed?

@Nicolas-MacBeth
Copy link
Contributor Author

It is needed @bogdandrutu ! I had just done it for early review and tweaking of the design/README. The full PR is done and can be found here: #453. That PR was a little massive, so I've broken it into 2 chunks, first part is here: #499. As soon as the first part is merged, I'll make the second.

To recap, I'm closing this PR, have a second huge PR with the entire receiver and finally have a third PR with a chunk of it (easier to review and merge). Thanks @james-bebbington

@james-bebbington
Copy link
Member

james-bebbington commented Jul 24, 2020

If this README is still up to date, we can keep this PR - it says "under development" so can be checked in before the rest of the code imo. I just noticed you had the same file committed in both PRs, and this one seems to be out of date.

@Nicolas-MacBeth
Copy link
Contributor Author

I'll close it since the README is out of date, and to not have too many PRs open at once!

bogdandrutu pushed a commit that referenced this pull request May 12, 2022
codeboten pushed a commit that referenced this pull request Nov 23, 2022
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.

4 participants